Fix propagation of global attributes to child datasets and variables - #72
Fix propagation of global attributes to child datasets and variables#72clarasb wants to merge 6 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
=======================================
Coverage 99.48% 99.48%
=======================================
Files 74 74
Lines 2927 2932 +5
Branches 425 425
=======================================
+ Hits 2912 2917 +5
Partials 15 15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
forman
left a comment
forman
left a comment
There was a problem hiding this comment.
Please fix:
- It is not clear, why the fix is correct. New code is also quite unintuitive.
- Missing unit tests
| rule_op.validate_datatree(context, node) | ||
|
|
||
| if node.datatree.is_leaf: | ||
| # Inherit attrs from the parent datatree into the child dataset |
There was a problem hiding this comment.
This just explains what the code does, but this is obvious. Please answer the question why this is done. What issues is solved by copying attributes.
| ) | ||
| else: | ||
| for name, datatree in node.datatree.children.items(): | ||
| # Inherit attrs from the parent datatree into the child datatree |
| - Fixed passing of global datatree attributes to children: attributes defined | ||
| on parent datatrees are now inherited by all descendants. (#63) |
There was a problem hiding this comment.
Why? To me this sounds like an xarray bug for which we provide a workaround here. Why do we have to inherit them manually? To me it seems we are copying attributes from a parent group into a child group - that would be wrong as it does not reflect the true physical structure of the Zarr dataset.
|
As #73 is merged, this PR is now obsolete and can be closed. |
2 participants
This PR fixes passing of global datatree attributes to children. Attributes defined on parent datatrees are now inherited by all descendants.
Closes #63.
Checklist (strike out non-applicable):
CHANGES.mdCHANGES.mddocs/*