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

Change exception to warning if xy_scale_size metadata is absent #182

Closed
wants to merge 1 commit into from

Conversation

talonchandler
Copy link
Contributor

Fixes #180.

Also, defaults to 1.0 um if metadata is absent.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Aug 24, 2023

We can also modify the info command to catch the error and show 'not available'. The main concern I have is user code depending on the old behavior will silently use the wrong value if it's not executed in an interactive environment.

@talonchandler
Copy link
Contributor Author

Possibly naive, but is it possible for users to depend on the old behavior if it raises AttributeError when the metadata is absent?

I also think that setting to default 1.0 is the right thing to do here since that's what happens for the ome-zarr files with identity transformations, right?

FWIW this code was introduced ~3 months ago in #118.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Aug 25, 2023

is it possible for users to depend on the old behavior if it raises AttributeError when the metadata is absent?

Something like:

try:
    px_size = reader.xy_scale_size
except AttributeError:
    px_size = json.load("some/custom/metadata.txt")["pixel_size"]

I don't know anyone who is doing this, but if the purpose is just to fix #180 there is also no need to break APIs.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Mar 19, 2024

Closing in favor of #185 and will be shipped in v0.2.0.

Thanks @talonchandler!

@ziw-liu ziw-liu closed this Mar 19, 2024
@ziw-liu ziw-liu deleted the fix-mm-scale2 branch June 11, 2024 23:29
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.

iohub info fails on MM datasets without xy_pixel_size
2 participants