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.

Sign up for GitHub

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

Jump to bottom

Zarr: drop "source" and "original_shape" from encoding #7500

Merged
merged 4 commits into from
Feb 7, 2023
Merged

Zarr: drop "source" and "original_shape" from encoding #7500

merged 4 commits into from
Feb 7, 2023

Conversation

observingClouds
Copy link
Contributor

@observingClouds observingClouds commented Feb 4, 2023 •
edited
Loading

@github-actions github-actions bot added io topic-backends topic-zarr Related to zarr storage library labels Feb 4, 2023
@max-sixty
Copy link
Collaborator

Thanks @observingClouds !

I wasn't paying attention to the original issue so this might be missing things — is there any concern that either of those are on the original zarr and we'd be removing them?

@observingClouds
Copy link
Contributor Author

I'm just mimicking the netCDF4 driver here. Maybe one could use less probable attributes than source? Maybe adding a prefix like xarray_ to those attributes? I'm open to suggestions.

@max-sixty
Copy link
Collaborator

Yes, very much makes sense to copy the netCDF behavior.

I'll let @dcherian comment on the tradeoff. Either warning when reading or adding a prefix sound reasonable (but also probably can be separate PRs, this still seems like an improvement)

@dcherian dcherian requested a review from jhamman February 6, 2023 17:13
@dcherian dcherian changed the title Fix 7129 Zarr: drop "source" and "original_shape" from encoding Feb 6, 2023
jhamman
jhamman approved these changes Feb 7, 2023
View reviewed changes
Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

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

Looks good! Just one small typo in the changelog.

@observingClouds @jhamman
Fix typo
dcd1383 
Co-authored-by: Joe Hamman <[email protected]>
@dcherian dcherian merged commit 5b6d757 into pydata:main Feb 7, 2023
@observingClouds observingClouds deleted the fix_7129 branch February 7, 2023 04:39
dcherian added a commit to dcherian/xarray that referenced this pull request Feb 7, 2023
@dcherian
Merge remote-tracking branch 'upstream/main' into whats-new-2023.02.0
c7c20d7 
* upstream/main:
  Update apply_ufunc output_sizes error message (pydata#7509)
  Zarr: drop "source" and  "original_shape" from encoding (pydata#7500)
  [pre-commit.ci] pre-commit autoupdate (pydata#7507)
dcherian added a commit to bzah/xarray that referenced this pull request Feb 28, 2023
@dcherian
Merge remote-tracking branch 'upstream/main' into fix/dask_indexing
97fa188 
* upstream/main: (291 commits)
  Update error message when saving multiindex (pydata#7475)
  DOC: cross ref the groupby tutorial (pydata#7555)
  [pre-commit.ci] pre-commit autoupdate (pydata#7543)
  supress namespace_package deprecation warning (doctests) (pydata#7548)
  [skip-ci] Add PDF of Xarray logo (pydata#7530)
  Support complex arrays in xr.corr (pydata#7392)
  clarification for thresh arg of dataset.dropna() (pydata#7481)
  [pre-commit.ci] pre-commit autoupdate (pydata#7524)
  Require to explicitly defining optional dimensions such as hue and markersize (pydata#7277)
  Use plt.rc_context for default styles (pydata#7318)
  Update HOW_TO_RELEASE.md (pydata#7512)
  Update whats-new for dev (pydata#7511)
  Fix whats-new for 2023.02.0 (pydata#7506)
  Update apply_ufunc output_sizes error message (pydata#7509)
  Zarr: drop "source" and  "original_shape" from encoding (pydata#7500)
  [pre-commit.ci] pre-commit autoupdate (pydata#7507)
  Whats-new for 2023.03.0
  Add `inclusive` argument to `cftime_range` and `date_range` and deprecate `closed` argument (pydata#7373)
  Make text match code example (pydata#7499)
  Remove Dask single-threaded setting in tests (pydata#7489)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhamman jhamman jhamman approved these changes

Assignees
No one assigned
Labels
io topic-backends topic-zarr Related to zarr storage library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dataset encodings 'source' and 'original_shape' are not dropped in zarr backend
4 participants
@observingClouds @max-sixty @jhamman @dcherian