Feature/add writers #9
Conversation
| @@ -29,6 +29,14 @@ classifiers = [ | |||
| dynamic = ["version"] | |||
| dependencies = [ | |||
| "bioio-base>=0.1.1", | |||
| "dask[array]>=2021.4.1", | |||
There was a problem hiding this comment.
Lots of dependencies added here that are specific to the writers. Does it seem worth it to move these to an extra so they are installable like bioio[writers] instead of by default or is that just added complexity for not much gain?
There was a problem hiding this comment.
dask is an interesting one - I can see many projects importing dask on their own too. Personally I don't have a strong opinion other than "keep it simple" - start with this and we can change things later if we need to separate it into an extra.
| "write_shape, write_dim_order, read_shape", | ||
| [ | ||
| # TODO: Failing currently, needs work, | ||
| # see https://github.com/bioio-devs/bioio/issues/10 |
There was a problem hiding this comment.
These two commented out tests pass on aicsimageio (technically) see the issue for why this was commented out.
bioio/tests/conftest.py
Outdated
| LOCAL_RESOURCES_DIR = pathlib.Path(__file__).parent / "resources" | ||
| LOCAL_RESOURCES_WRITE_DIR = pathlib.Path(__file__).parent / "writer_products" | ||
|
|
||
|
|
||
| def get_resource_full_path(filename: str) -> typing.Union[str, pathlib.Path]: | ||
| return LOCAL_RESOURCES_DIR / filename | ||
| def pytest_sessionstart(session: pytest.Session) -> None: |
There was a problem hiding this comment.
Actually rethinking this I think I'll change this to use tempdir. Maybe we can write out an example.txt to tempdir too and avoid having any files download for this package thus far.
pyproject.toml
Outdated
| "fsspec>=2022.8.0", | ||
| "imageio[ffmpeg]>=2.11.0,<2.28.0", | ||
| "numpy>=1.16.0,<2.0.0", | ||
| "ome-types>=0.3.3", |
There was a problem hiding this comment.
I think ome-types 0.3.4, but also see this https://forum.image.sc/t/ome-types-rewritten-requesting-alpha-testers/83174
There was a problem hiding this comment.
There was a problem hiding this comment.
while that migration guide looks like a lot, you should also know that I routinely test against aicsimageio ... and you should be able to use v0.4.0 without any errors (albeit plenty of name change warnings which that migration guide would help with).
I don't think it should cause any ecosystem difficulties if you pin to >0.4.0 ... since v0.4.0 should work for any libraries that haven't updated
There was a problem hiding this comment.
I basically agree, we might as well go with the latest if there are no obvious problems 🚀
There was a problem hiding this comment.
happy to help with that. do you want me to open a PR to remove all the warnings?
There was a problem hiding this comment.
btw... i'm also working on pydantic2 support (tlambert03/ome-types#205) ... and it's speedy!
(though, I don't think you should pin pydantic yet)
There was a problem hiding this comment.
@SeanLeRoy should chime in, but I'll just say that one other option is to defer this dependency version update till we move further along on getting bioio ready. But I'm comfy merging such a change both here and aicsimageio anytime
There was a problem hiding this comment.
yep, sounds good either way. just ping me if I can be of help
There was a problem hiding this comment.
I'm willing to try out migrating to ome-types>=0.4.0. I'll give it a shot as part of this and update here with my findings though it seems like it should be an easy migration
There was a problem hiding this comment.
Seemed easy enough 👍 , looks like as part of this lxml can be dropped as a direct dependency of this package and installed as an extra from ome-types which I did in #1dcd7ae
Link to Relevant Issue
This pull request resolves #3
Description of Changes
This adds the writers from
aicsimageio. The major difference between the writers and tests I copied fromaicsimageiois that these no longer rely onDefaultReaderorOmeTiffReaderso I had to useimageioandtifffilemore directly in the related writer and tests.Testing
I tested these against the unit tests and locally writing out some files.