Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add spatialdata testdata #10

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

melonora
Copy link

@melonora melonora commented Feb 6, 2024

This PR allows the generation of a SpatialData object at test time. This is done by using a pytest.fixture. Here, tmp_path is a fixture from _pytest.tmpdir.

It contains all the components currently present in SpatialData. More information here: github and scverse

SpatialData itself builds on top of OME-NGFF.

Copy link
Member

@cmungall cmungall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

120 files seems like a lot...

@sneakers-the-rat
Copy link

can we generate this with blobs().write() at test time so we can keep it up to date with however spatialdata may update its format as well?

also what is the fate of this package anyway? should format tests go in the main test suite or here? on the one hand gathering all the array tests here would be nice, on the other hand if we're doing the work in the main repo (rather than importing from here) this could get desync'd

@melonora
Copy link
Author

This would be possible at test time as far as I am aware.

@cmungall
Copy link
Member

@melonora do you want to modify the PR or make a new one?

@melonora
Copy link
Author

I can modify the PR. Is it ok to add spatialdata as a dependency in order to create the files at test time?

@cmungall
Copy link
Member

cmungall commented Feb 24, 2024 via email

Copy link
Member

@cmungall cmungall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unneeded files

@melonora
Copy link
Author

I am trying to update the PR, just with poetry resolving the dependencies seems to take forever. This does not seem to be due to a conflict arising. I already adjusted the python dependency to >=3.9,<3.13 to comply with the xarray requirements.

@melonora
Copy link
Author

@cmungall I think this should be good to go now. I made it so that the data can be created on the fly by using a fixture. The demo test demonstrates this.

@cmungall cmungall requested a review from rly February 27, 2024 19:13
@@ -7,13 +7,11 @@ license = "BSD-3"
readme = "README.md"

[tool.poetry.dependencies]
python = "^3.9"
python = ">=3.9,<3.13"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to get this to work with ^3.9.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment no due to xarray. The dependencies couldn't be resolved when using ^3.9 with the xarray version in SpatialData

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe let me retry because this was before merge

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no due to following:

Check your dependencies Python requirement: The Python requirement can be specified via the `python` or `markers` properties
    
    For xarray-dataclasses, a possible solution would be to set the `python` property to ">=3.9,<3.13"

@sneakers-the-rat
Copy link

I like what im seeing here. Is the plan to use this repo for extended testing of the generators in the main linkml repo? Im mostly finished with the pure lists of lists implementation and am about to start the more advanced one that can hook into array libraries, and it would be cool to be testing on real data from a real format like this

@melonora
Copy link
Author

I like what im seeing here. Is the plan to use this repo for extended testing of the generators in the main linkml repo? Im mostly finished with the pure lists of lists implementation and am about to start the more advanced one that can hook into array libraries, and it would be cool to be testing on real data from a real format like this

Would you like fixtures for the individual components as well? Because that can be added too.

@sneakers-the-rat
Copy link

Idk exactly what would be good yet, but will revisit when it's time to test the pydanticgen implementation of arrays

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants