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

Ome ngff pattonw #361

Merged
merged 12 commits into from
Jan 23, 2025
Merged

Ome ngff pattonw #361

merged 12 commits into from
Jan 23, 2025

Conversation

mzouink
Copy link
Member

@mzouink mzouink commented Jan 9, 2025

@pattonw i added some tests

Still getting error resolution is not correct, expected (1, 2, 4), got (1, 1, 1)

@mzouink mzouink mentioned this pull request Jan 9, 2025
mzouink and others added 5 commits January 19, 2025 13:35
Thanks for this test. I found a couple problems:
1) `funlib.persistence` uses `voxel_size` instead of `resolution` by
default. (fixed)
2) The `funlib.persistence` `open_ome_ds` function had a bug in it
resulting in an "Attribute does not exist" error (fixed)
3) The metadata provided in the test is apparently invalid. `iohub`
throws this error: "WARNING root:ngff.py:291 Zarr group at does not have
valid metadata for <class 'iohub.ngff.Position'>". ( not yet fixed )

Probably not worth doing for the test, but this can be configured in
multiple ways (using python function `set_default_metadata_format` from
`funlib.persistence.arrays.metadata`, or using config files. One of :
```
LOCAL_PATHS = [Path("pyproject.toml"), Path("funlib_persistence.toml")]
USER_PATHS = [
    Path.home() / ".config" / "funlib_persistence" / "funlib_persistence.toml"
]
GLOBAL_PATHS = [Path("/etc/funlib_persistence/funlib_persistence.toml")]
```
tests.py Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

@pattonw i think this file is wrongly placed

@mzouink
Copy link
Member Author

mzouink commented Jan 21, 2025

@pattonw can you please check the tests.py file so we can merge

@pattonw
Copy link
Contributor

pattonw commented Jan 21, 2025

Yes, I don't remember adding those tests. I'm not sure where they came from. I'll check it out and see if we need them

@mzouink mzouink merged commit 8c5ae81 into main Jan 23, 2025
4 checks passed
@mzouink mzouink deleted the ome-ngff-pattonw branch January 23, 2025 19:01
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