-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Silence upstream Zarr warnings #9920
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Now that we don't fail on the warnings, there are upstream Zarr failures e.g. zarr-developers/zarr-python#2627 |
dcherian
added a commit
to dcherian/xarray
that referenced
this pull request
Mar 19, 2025
* main: (63 commits) Fix zarr upstream tests (pydata#9927) Update pre-commit hooks (pydata#9925) split out CFDatetimeCoder, deprecate use_cftime as kwarg (pydata#9901) dev whats-new (pydata#9923) Whats-new 2025.01.0 (pydata#9919) Silence upstream Zarr warnings (pydata#9920) time coding refactor (pydata#9906) fix warning from scipy backend guess_can_open on directory (pydata#9911) Enhance and move ISO-8601 parser to coding.times (pydata#9899) Edit serialization error message (pydata#9916) friendlier error messages for missing chunk managers (pydata#9676) Bump codecov/codecov-action from 5.1.1 to 5.1.2 in the actions group (pydata#9915) Rewrite interp to use `apply_ufunc` (pydata#9881) Skip dask rolling (pydata#9909) Explicitly configure ReadTheDocs build to use conf.py (pydata#9908) Cache pre-existing Zarr arrays in Zarr backend (pydata#9861) Optimize idxmin, idxmax with dask (pydata#9800) remove unused "type: ignore" comments in test_plot.py (fixed in matplotlib 3.10.0) (pydata#9904) move scalar-handling logic into `possibly_convert_objects` (pydata#9900) Add missing DataTree attributes to docs (pydata#9876) ...
max-sixty
added a commit
to max-sixty/xarray
that referenced
this pull request
Sep 9, 2025
BREAKING CHANGE: Change keep_attrs default from False to True This changes the default behavior of xarray operations to preserve attributes by default, which better aligns with user expectations and scientific workflows where metadata preservation is critical. Migration guide: - To restore previous behavior globally: xr.set_options(keep_attrs=False) - To restore for specific operations: use keep_attrs=False parameter - Alternative: use .drop_attrs() method after operations Closes pydata#3891, pydata#4510, pydata#9920
max-sixty
added a commit
that referenced
this pull request
Oct 8, 2025
* feat: Preserve attributes by default in all operations BREAKING CHANGE: Change keep_attrs default from False to True This changes the default behavior of xarray operations to preserve attributes by default, which better aligns with user expectations and scientific workflows where metadata preservation is critical. Migration guide: - To restore previous behavior globally: xr.set_options(keep_attrs=False) - To restore for specific operations: use keep_attrs=False parameter - Alternative: use .drop_attrs() method after operations Closes #3891, #4510, #9920 * Fix Dataset.map to properly handle coordinate attrs when keep_attrs=False The merge incorrectly preserved coordinate attributes even when keep_attrs=False. Now coordinates have their attrs cleared when keep_attrs=False, consistent with data variables. * Optimize Dataset.map coordinate attribute handling - When keep_attrs=True: restore attrs from original coords (func may have dropped them) - When keep_attrs=False: clear all attrs - More efficient than previous implementation * Simplify Dataset.map attribute handling code Group attribute operations by keep_attrs value for cleaner, more readable code with identical functionality. * Remove temporal 'now' references from comments Per Stefan's review, remove 'now' from comments that describe behavior changes, as these become stale over time. Replace with timeless descriptions that simply state the current behavior. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address remaining review comments from Stefan - Remove PR-specific comment from variable.py that only makes sense in context - Remove redundant comment from test_computation.py - Clarify comment in test_dataarray.py about argmin preserving attrs * Use drop_conflicts for binary operations attribute handling Instead of only preserving the left operand's attributes, binary operations now combine attributes from both operands using the drop_conflicts strategy: - Matching attributes (same key, same value) are kept - Conflicting attributes (same key, different values) are dropped - Non-conflicting attributes from both operands are preserved This provides more intuitive behavior when combining data with partially overlapping metadata. * Fix binary ops attrs: only merge when both operands have attrs For backward compatibility, when one operand has no attributes (None), keep the left operand's attributes instead of merging. This maintains the existing test expectations while still providing drop_conflicts behavior when both operands have attributes. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix binary ops attrs handling when operands have no attrs When one operand has no attributes (either None or empty dict), the result should have no attributes. This maintains backward compatibility and fixes test_1d_math failures. The issue was compounded by a side effect in the attrs property getter that mutates _attrs from None to {} when accessed. * Simplify binary ops attrs handling Use attrs property directly instead of checking _attrs, since the property normalizes None to {}. This simplifies the logic while maintaining the same behavior. * Clarify comments about attrs handling differences Variable uses None for no attrs, Dataset uses {} for no attrs. Updated comments to make this distinction clear. * Implement true drop_conflicts behavior for binary operations Previously we were dropping all attrs if either operand had no attrs. Now we properly merge attrs and only drop conflicting ones, which is what drop_conflicts should do: - If one has {"a": 1} and other has {}, result is {"a": 1} - If one has {"a": 1} and other has {"a": 2}, result is {} - If one has {"a": 1} and other has {"b": 2}, result is {"a": 1, "b": 2} Updated tests to reflect this correct behavior. * Remove unnecessary conversion of {} to None Variable constructor already normalizes empty dict to None internally, so the explicit conversion is redundant. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #9898
whats-new.rstapi.rst