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

nested separator in to_ngff_zarr, fixed napari display problem #46

Merged
merged 1 commit into from
Jul 24, 2023

Conversation

GFleishman
Copy link

@GFleishman GFleishman commented Jul 17, 2023

This fixes #45

I noticed calls to to_zarr in the to_multiscales function as well, but I think they're just cache for large files and not meant as ome-ngff-zarr outputs so I did not add a dimension separator for them.

You can also specify the dimension separator when you create the array (i.e. zarr.creation.open_array) but since you don't always do this explicitly I let the dask.array.to_zarr function do it.

Not sure if you want the dimension separator to be a user settable thing instead of hardcoded like this? If it's going to be standard practice for ome-ngff-zarr to use '/' and if some programs won't even support '.' then maybe compulsory like this is better, but your choice.

@GFleishman
Copy link
Author

I have no idea how adding a keyword argument to an internal function would cause doc build to fail?

@thewtex thewtex merged commit 0595a3e into thewtex:main Jul 24, 2023
Copy link
Owner

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

@GFleishman thank you so much for a addressing this!! 💯 🥇

@joshmoore added this to the OME-Zarr spec and the functionality to Zarr Python -- he can correct me, but I believe it is not the default for backwards-compatibility reasons.

@thewtex
Copy link
Owner

thewtex commented Jul 24, 2023

I have no idea how adding a keyword argument to an internal function would cause doc build to fail?

@GFleishman I likely added the initial docs after you created the branch for this PR.

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.

Image data not displayed in napari
2 participants