Preserve dimension separator metadata when resizing arrays #1540

ziw-liu · 2023-10-05T19:38:50Z

When constructing the updated metadata for a resized V2 array, preserve the dimension_separator field so it is not reverted to the default after resizing.

Fixes #1533.

TODO:

Add unit tests and/or doctests in docstrings
Add docstrings and API docs for any new/modified user-facing classes and functions
New/modified features documented in docs/tutorial.rst
Changes documented in docs/release.rst
GitHub Actions have all passed
Test coverage is 100% (Codecov passes)

codecov · 2023-10-24T19:32:05Z

Codecov Report

Merging #1540 (871d0a6) into main (5eb737b) will decrease coverage by 0.01%.
The diff coverage is 85.71%.

Additional details and impacted files

@@             Coverage Diff             @@
##              main    #1540      +/-   ##
===========================================
- Coverage   100.00%   99.99%   -0.01%     
===========================================
  Files           37       37              
  Lines        14735    14740       +5     
===========================================
+ Hits         14735    14739       +4     
- Misses           0        1       +1

Files	Coverage Δ
zarr/core.py	`100.00% <100.00%> (ø)`
zarr/tests/test_core.py	`99.95% <83.33%> (-0.05%)`	⬇️

MSanKeys963 · 2023-10-24T21:02:38Z

Hi @ziw-liu, thanks for sending the PR.

I have requested reviews from the Zarr-Python devs. Additionally, if anyone from the @zarr-developers/python-core-devs can review the PR, I'd be thankful to them.

d-v-b · 2023-10-24T21:18:30Z

this looks good! codecov is complaining about a coverage decrease in tests, that seems a bit weird but I don't think that stems from the changes in your PR. There's a lot of whitespace changes that also seem odd, i'm going to see if running our code formatters (ruff and black) clears that up.

ziw-liu · 2023-10-24T22:37:01Z

@d-v-b Thanks for the review!

There's a lot of whitespace changes that also seem odd, i'm going to see if running our code formatters (ruff and black) clears that up.

I ran black on that file. The original style (empty line after function definition) is not valid per black rules but somehow passed CI before this. Please feel free to revert those changes.

d-v-b · 2023-10-25T19:55:31Z

@zarr-developers/core-devs any objections to merging this?

joshmoore · 2023-10-30T17:00:57Z

zarr/core.py

@@ -341,7 +340,14 @@ def _flush_metadata_nosync(self):
            filters=filters_config,
        )
        if getattr(self._store, "_store_version", 2) == 2:
-            meta.update(dict(chunks=self._chunks, dtype=self._dtype, order=self._order))
+            meta.update(


Thanks, @ziw-liu! Moving forward, I do wonder if we don't want to encapsulate all of these items to prevent this type of error.

…lopers#1540) * preserve dimension separator when resizing arrays * test dimension separator metadata after resizing * document the change * Update release.rst --------- Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>

zarr-developers/zarr-python#1540

…185) * wip: draft mmstack ome-tiff fov * MM FOV base class * tests * bump tifffile * comment * fix indent after rebase * use get default * test pixel indexing * set MM metadata * style * update dependencies * add xarray * move old readers to the `_deprecated` namespace * uapi for ndtiff * refactor test setup to parametrize by dataset use globals instead of fixtures since parametrization happens before fixture evaluation * convert mmstack * fix and test chunking * fix metadata conversion and test ndtiff * update cli * fix scaling * test 1.4 and incomplete ome-tiffs * move reader tests * deprecate reader tests * update deprecated tests * update ngff tests * isort * update black target to 3.10 * lint * fix download paths * update docs references and theme * untrack autogenerated file * ignore execution time file * add github icon * update docstring * update docstring * show channel names and chunk size in info * print plate chunk size if verbose * fallback for pixel size * remove log level setting * do not filter logs and warnings in reader * avoid root logger * isort * set default logging level to INFO * format docstring * improve conversion messages * black * fix ome-tiff channel name indexing * fix ndtiff channel name indexing * update converter test * remove use of os.path in `reader` * expand _check_ndtiff checks * fix iteration * fix python 3.10 using `Path.glob(*/)` to get subdirs was added in 3.11 * bump zarr version to include resizing fix zarr-developers/zarr-python#1540 * fix cli default * set log level with an environment variable * fix unset * catch non-existent page * implement fallback for incomplete channel names workaround for the Dragonfly microscope where the multi-camera setup only has one channel name written * add debug logs * handle virtual frames * try reading pages from TiffFile directly * filter error logs about ImageJ metadata being broken this is a known MM limitation when writing OME-TIFFs * fix regex * remove use of os.path in `convert.py` * better channel indexing in `_get_summary_metadata` * style * safer NoneType check * private default axis names for NDTiff * update documentation to reflect new entry point * add repr to MM FOV and dataset types * rename mm_meta and expose summary metadata * add MicroManagerFOVMapping.root * add MicroManagerFOVMapping.zyx_scale * add warning log for failed position grid * fix grid layout * suppress hypothesis flakiness * different health check suppression --------- Co-authored-by: Ivan Ivanov <ivan.ivanov@czbiohub.org>

commit fac2c13 Author: Ivan Ivanov <ivan.ivanov@czbiohub.org> Date: Tue Apr 9 11:25:36 2024 -0700 Fix bug reading dragonfly acquisitions (#215) * fix bug reading dragonfly acquisitions * fix typo * style * bugfix commit 0c6984e Author: Ivan Ivanov <ivan.ivanov@czbiohub.org> Date: Mon Mar 11 12:35:51 2024 -0700 Fix bug determining number of rows and cols (#214) * fix bug determining number of rows and cols * add another XY Stage variation * add docs and fix style commit 3ab89ba Author: Ziwen Liu <67518483+ziw-liu@users.noreply.github.com> Date: Mon Mar 4 11:02:49 2024 -0800 Universal API implementations for Micro-Manager OME-TIFF and NDTiff (#185) * wip: draft mmstack ome-tiff fov * MM FOV base class * tests * bump tifffile * comment * fix indent after rebase * use get default * test pixel indexing * set MM metadata * style * update dependencies * add xarray * move old readers to the `_deprecated` namespace * uapi for ndtiff * refactor test setup to parametrize by dataset use globals instead of fixtures since parametrization happens before fixture evaluation * convert mmstack * fix and test chunking * fix metadata conversion and test ndtiff * update cli * fix scaling * test 1.4 and incomplete ome-tiffs * move reader tests * deprecate reader tests * update deprecated tests * update ngff tests * isort * update black target to 3.10 * lint * fix download paths * update docs references and theme * untrack autogenerated file * ignore execution time file * add github icon * update docstring * update docstring * show channel names and chunk size in info * print plate chunk size if verbose * fallback for pixel size * remove log level setting * do not filter logs and warnings in reader * avoid root logger * isort * set default logging level to INFO * format docstring * improve conversion messages * black * fix ome-tiff channel name indexing * fix ndtiff channel name indexing * update converter test * remove use of os.path in `reader` * expand _check_ndtiff checks * fix iteration * fix python 3.10 using `Path.glob(*/)` to get subdirs was added in 3.11 * bump zarr version to include resizing fix zarr-developers/zarr-python#1540 * fix cli default * set log level with an environment variable * fix unset * catch non-existent page * implement fallback for incomplete channel names workaround for the Dragonfly microscope where the multi-camera setup only has one channel name written * add debug logs * handle virtual frames * try reading pages from TiffFile directly * filter error logs about ImageJ metadata being broken this is a known MM limitation when writing OME-TIFFs * fix regex * remove use of os.path in `convert.py` * better channel indexing in `_get_summary_metadata` * style * safer NoneType check * private default axis names for NDTiff * update documentation to reflect new entry point * add repr to MM FOV and dataset types * rename mm_meta and expose summary metadata * add MicroManagerFOVMapping.root * add MicroManagerFOVMapping.zyx_scale * add warning log for failed position grid * fix grid layout * suppress hypothesis flakiness * different health check suppression --------- Co-authored-by: Ivan Ivanov <ivan.ivanov@czbiohub.org>

* wip: draft mmstack ome-tiff fov * MM FOV base class * tests * bump tifffile * comment * fix indent after rebase * use get default * test pixel indexing * set MM metadata * style * update dependencies * add xarray * move old readers to the `_deprecated` namespace * uapi for ndtiff * refactor test setup to parametrize by dataset use globals instead of fixtures since parametrization happens before fixture evaluation * convert mmstack * fix and test chunking * fix metadata conversion and test ndtiff * update cli * fix scaling * test 1.4 and incomplete ome-tiffs * move reader tests * deprecate reader tests * update deprecated tests * update ngff tests * isort * update black target to 3.10 * lint * fix download paths * update docs references and theme * untrack autogenerated file * ignore execution time file * add github icon * update docstring * update docstring * show channel names and chunk size in info * print plate chunk size if verbose * fallback for pixel size * remove log level setting * do not filter logs and warnings in reader * avoid root logger * isort * set default logging level to INFO * format docstring * improve conversion messages * black * fix ome-tiff channel name indexing * fix ndtiff channel name indexing * update converter test * remove use of os.path in `reader` * expand _check_ndtiff checks * fix iteration * fix python 3.10 using `Path.glob(*/)` to get subdirs was added in 3.11 * bump zarr version to include resizing fix zarr-developers/zarr-python#1540 * fix cli default * set log level with an environment variable * fix unset * catch non-existent page * implement fallback for incomplete channel names workaround for the Dragonfly microscope where the multi-camera setup only has one channel name written * add debug logs * handle virtual frames * try reading pages from TiffFile directly * filter error logs about ImageJ metadata being broken this is a known MM limitation when writing OME-TIFFs * fix regex * remove use of os.path in `convert.py` * better channel indexing in `_get_summary_metadata` * style * safer NoneType check * private default axis names for NDTiff * sort metadata keys * Squashed commit of the following: commit fac2c13 Author: Ivan Ivanov <ivan.ivanov@czbiohub.org> Date: Tue Apr 9 11:25:36 2024 -0700 Fix bug reading dragonfly acquisitions (#215) * fix bug reading dragonfly acquisitions * fix typo * style * bugfix commit 0c6984e Author: Ivan Ivanov <ivan.ivanov@czbiohub.org> Date: Mon Mar 11 12:35:51 2024 -0700 Fix bug determining number of rows and cols (#214) * fix bug determining number of rows and cols * add another XY Stage variation * add docs and fix style commit 3ab89ba Author: Ziwen Liu <67518483+ziw-liu@users.noreply.github.com> Date: Mon Mar 4 11:02:49 2024 -0800 Universal API implementations for Micro-Manager OME-TIFF and NDTiff (#185) * wip: draft mmstack ome-tiff fov * MM FOV base class * tests * bump tifffile * comment * fix indent after rebase * use get default * test pixel indexing * set MM metadata * style * update dependencies * add xarray * move old readers to the `_deprecated` namespace * uapi for ndtiff * refactor test setup to parametrize by dataset use globals instead of fixtures since parametrization happens before fixture evaluation * convert mmstack * fix and test chunking * fix metadata conversion and test ndtiff * update cli * fix scaling * test 1.4 and incomplete ome-tiffs * move reader tests * deprecate reader tests * update deprecated tests * update ngff tests * isort * update black target to 3.10 * lint * fix download paths * update docs references and theme * untrack autogenerated file * ignore execution time file * add github icon * update docstring * update docstring * show channel names and chunk size in info * print plate chunk size if verbose * fallback for pixel size * remove log level setting * do not filter logs and warnings in reader * avoid root logger * isort * set default logging level to INFO * format docstring * improve conversion messages * black * fix ome-tiff channel name indexing * fix ndtiff channel name indexing * update converter test * remove use of os.path in `reader` * expand _check_ndtiff checks * fix iteration * fix python 3.10 using `Path.glob(*/)` to get subdirs was added in 3.11 * bump zarr version to include resizing fix zarr-developers/zarr-python#1540 * fix cli default * set log level with an environment variable * fix unset * catch non-existent page * implement fallback for incomplete channel names workaround for the Dragonfly microscope where the multi-camera setup only has one channel name written * add debug logs * handle virtual frames * try reading pages from TiffFile directly * filter error logs about ImageJ metadata being broken this is a known MM limitation when writing OME-TIFFs * fix regex * remove use of os.path in `convert.py` * better channel indexing in `_get_summary_metadata` * style * safer NoneType check * private default axis names for NDTiff * update documentation to reflect new entry point * add repr to MM FOV and dataset types * rename mm_meta and expose summary metadata * add MicroManagerFOVMapping.root * add MicroManagerFOVMapping.zyx_scale * add warning log for failed position grid * fix grid layout * suppress hypothesis flakiness * different health check suppression --------- Co-authored-by: Ivan Ivanov <ivan.ivanov@czbiohub.org> * black * bugfix --------- Co-authored-by: Ziwen Liu <67518483+ziw-liu@users.noreply.github.com>

ziw-liu added 3 commits October 5, 2023 12:19

preserve dimension separator when resizing arrays

3e16d3c

test dimension separator metadata after resizing

4cb2a49

document the change

c952c73

MSanKeys963 requested review from rabernat, jakirkham and d-v-b October 24, 2023 20:59

d-v-b approved these changes Oct 25, 2023

View reviewed changes

d-v-b added 3 commits October 27, 2023 00:04

Merge branch 'main' into fix-resize-dimension-separator

97a8ca5

Update release.rst

4a21fbe

Merge branch 'main' into fix-resize-dimension-separator

871d0a6

d-v-b enabled auto-merge (squash) October 26, 2023 22:26

d-v-b merged commit 1ed37f5 into zarr-developers:main Oct 26, 2023
20 of 21 checks passed

joshmoore reviewed Oct 30, 2023

View reviewed changes

ziw-liu deleted the fix-resize-dimension-separator branch October 30, 2023 17:47

ziw-liu mentioned this pull request Feb 13, 2024

support ome-ngff on s3 czbiohub-sf/iohub#209

Open

ziw-liu added a commit to czbiohub-sf/iohub that referenced this pull request Feb 15, 2024

bump zarr version to include resizing fix

f6a5bd7

zarr-developers/zarr-python#1540

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preserve dimension separator metadata when resizing arrays #1540

Preserve dimension separator metadata when resizing arrays #1540

ziw-liu commented Oct 5, 2023 •

edited

Loading

codecov bot commented Oct 24, 2023 •

edited

Loading

MSanKeys963 commented Oct 24, 2023

d-v-b commented Oct 24, 2023

ziw-liu commented Oct 24, 2023

d-v-b commented Oct 25, 2023

joshmoore Oct 30, 2023

Preserve dimension separator metadata when resizing arrays #1540

Preserve dimension separator metadata when resizing arrays #1540

Conversation

ziw-liu commented Oct 5, 2023 • edited Loading

codecov bot commented Oct 24, 2023 • edited Loading

Codecov Report

MSanKeys963 commented Oct 24, 2023

d-v-b commented Oct 24, 2023

ziw-liu commented Oct 24, 2023

d-v-b commented Oct 25, 2023

joshmoore Oct 30, 2023

Choose a reason for hiding this comment

ziw-liu commented Oct 5, 2023 •

edited

Loading

codecov bot commented Oct 24, 2023 •

edited

Loading