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

Migrate datatree mapping.py #8948

Merged
merged 2 commits into from
Apr 17, 2024

Conversation

owenlittlejohns
Copy link
Contributor

This PR continues the overall work of migrating DataTree into xarray.

datatree_mapping.py is the renamed version of mapping.py from the datatree repository.

  • Closes migration step for mapping.py Track merging datatree into xarray #8572
  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
  • New functions/methods are listed in api.rst

@@ -4,10 +4,9 @@
import sys
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomNicholas - there are no significant changes to the way map_over_subtree works in this PR. As discussed today, I've opened the PR, and am happy to make changes based on conversation here.

Copy link
Member

Choose a reason for hiding this comment

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

Okay looking at this again I can't see an obvious way to refactor it. I'm mostly just concerned that it actually makes sense to you @owenlittlejohns ?

If so then I suggest we spend the time thinking about #8949 instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomNicholas - yeah. @flamingbear and I have both spent a fair bit of time trying to grok this module. I don't claim that I've got a perfect understanding, but I feel fairly good on the whole. (The conversation yesterday that led to #8949 helped a bit, too)

Copy link
Member

Choose a reason for hiding this comment

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

Okay great. So I guess we merge this??

FYI if you want to chat about any of this over zoom outside of the weekly meeting I would be happy to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we're good to merge!

@TomNicholas TomNicholas added the topic-DataTree Related to the implementation of a DataTree class label Apr 16, 2024
@TomNicholas TomNicholas changed the title Das 2064 migrate mapping Migrate datatree mapping.py Apr 16, 2024
@TomNicholas TomNicholas merged commit 60f3e74 into pydata:main Apr 17, 2024
37 checks passed
@TomNicholas
Copy link
Member

Thank you!

@flamingbear flamingbear deleted the DAS-2064-migrate-mapping branch April 17, 2024 20:44
dcherian added a commit to djhoese/xarray that referenced this pull request Apr 18, 2024
* main:
  (feat): Support for `pandas` `ExtensionArray` (pydata#8723)
  Migrate datatree mapping.py (pydata#8948)
  Add mypy to dev dependencies (pydata#8947)
  Convert 360_day calendars by choosing random dates to drop or add (pydata#8603)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-DataTree Related to the implementation of a DataTree class
Projects
Development

Successfully merging this pull request may close these issues.

3 participants