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

Fix Zarr region transpose #8484

Merged
merged 4 commits into from
Nov 27, 2023
Merged

Conversation

max-sixty
Copy link
Collaborator

This wasn't working on an unregion-ed write; I think because new_var was being lost.

This wasn't working on an unregion-ed write; I think because `new_var` was being lost.
@github-actions github-actions bot added topic-backends topic-zarr Related to zarr storage library io labels Nov 25, 2023
@max-sixty
Copy link
Collaborator Author

CC @slevang , feedback welcome!

@max-sixty
Copy link
Collaborator Author

FYI I was getting a tb like:

File /opt/homebrew/lib/python3.9/site-packages/xarray/backends/api.py:1306, in dump_to_store(dataset, store, writer, encoder, encoding, unlimited_dims)
   1303 if encoder:
   1304     variables, attrs = encoder(variables, attrs)
-> 1306 store.store(variables, attrs, check_encoding, writer, unlimited_dims=unlimited_dims)

File /opt/homebrew/lib/python3.9/site-packages/xarray/backends/zarr.py:641, in ZarrStore.store(self, variables, attributes, check_encoding_set, writer, unlimited_dims)
    638     self.set_attributes(attributes)
    639     self.set_dimensions(variables_encoded, unlimited_dims=unlimited_dims)
--> 641 self.set_variables(
    642     variables_encoded, check_encoding_set, writer, unlimited_dims=unlimited_dims
    643 )
    644 if self._consolidate_on_close:
    645     zarr.consolidate_metadata(self.zarr_group.store)

File /opt/homebrew/lib/python3.9/site-packages/xarray/backends/zarr.py:761, in ZarrStore.set_variables(self, variables, check_encoding_set, writer, unlimited_dims)
    758     zarr_array.resize(new_shape)
    760 region = tuple(write_region[dim] for dim in dims)
--> 761 writer.add(v.data, zarr_array, region)

File /opt/homebrew/lib/python3.9/site-packages/xarray/backends/common.py:241, in ArrayWriter.add(self, source, target, region)
    239 else:
    240     if region:
--> 241         target[region] = source
    242     else:
    243         target[...] = source

File /opt/homebrew/lib/python3.9/site-packages/zarr/core.py:1495, in Array.__setitem__(self, selection, value)
   1493     self.vindex[selection] = value
   1494 elif is_pure_orthogonal_indexing(pure_selection, self.ndim):
-> 1495     self.set_orthogonal_selection(pure_selection, value, fields=fields)
   1496 else:
   1497     self.set_basic_selection(pure_selection, value, fields=fields)

File /opt/homebrew/lib/python3.9/site-packages/zarr/core.py:1684, in Array.set_orthogonal_selection(self, selection, value, fields)
   1681 # setup indexer
   1682 indexer = OrthogonalIndexer(selection, self)
-> 1684 self._set_selection(indexer, value, fields=fields)

File /opt/homebrew/lib/python3.9/site-packages/zarr/core.py:2011, in Array._set_selection(self, indexer, value, fields)
   2009     if not hasattr(value, "shape"):
   2010         value = np.asanyarray(value, like=self._meta_array)
-> 2011     check_array_shape("value", value, sel_shape)
   2013 # iterate over chunks in range
   2014 if (
   2015     not hasattr(self.chunk_store, "setitems")
   2016     or self._synchronizer is not None
   2017     or any(map(lambda x: x == 0, self.shape))
   2018 ):
   2019     # iterative approach

File /opt/homebrew/lib/python3.9/site-packages/zarr/util.py:560, in check_array_shape(param, array, shape)
    556     raise TypeError(
    557         "parameter {!r}: expected an array-like object, got {!r}".format(param, type(array))
    558     )
    559 if array.shape != shape:
--> 560     raise ValueError(
    561         "parameter {!r}: expected array with shape {!r}, got {!r}".format(
    562             param, shape, array.shape
    563         )
    564     )

ValueError: parameter 'value': expected array with shape (5, 10), got (10, 5)

I'm not really sure how the region kwarg was causing it to work — or possibly it's something different. Regardless, the test previously failed and now passes, so I think this is an improvement...

Copy link
Contributor

@slevang slevang left a comment

Choose a reason for hiding this comment

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

Good catch, thanks. I am also confused about how this was working even with region specified?

Only thing is you may want to rename the test, since this (unintentionally but correctly) suggested that the feature only works when explicitly passing region.

@max-sixty
Copy link
Collaborator Author

Only thing is you may want to rename the test, since this (unintentionally but correctly) suggested

:)

Done!

@max-sixty max-sixty merged commit d54c461 into pydata:main Nov 27, 2023
28 checks passed
@max-sixty max-sixty deleted the zarr-transpose-fix branch November 27, 2023 20:56
dcherian added a commit to rabernat/xarray that referenced this pull request Nov 29, 2023
* upstream/main:
  Raise an informative error message when object array has mixed types (pydata#4700)
  Start renaming `dims` to `dim` (pydata#8487)
  Reduce redundancy between namedarray and variable tests (pydata#8405)
  Fix Zarr region transpose (pydata#8484)
  Refine rolling_exp error messages (pydata#8485)
  Use numbagg for `ffill` by default (pydata#8389)
  Fix bug for categorical pandas index with categories with EA dtype (pydata#8481)
  Improve "variable not found" error message (pydata#8474)
  Add whatsnew for pydata#8475 (pydata#8478)
  Allow `rank` to run on dask arrays (pydata#8475)
  Fix mypy tests (pydata#8476)
  Use concise date format when plotting (pydata#8449)
  Fix `map_blocks` docs' formatting (pydata#8464)
  Consolidate `_get_alpha` func (pydata#8465)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io topic-backends topic-zarr Related to zarr storage library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants