Skip to content

modification methods on Coordinates #10318

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented May 13, 2025

For ease of use, this implements a few additional methods on Coordinates:

  • rename_dims
  • rename_vars
  • drop_vars
  • coords1 | coords2 as an alias of coords1.merge(coords2)

These currently forward to the implementation in Dataset, but will be the natural place after refactoring Coordinates to not wrap a Dataset.


  • towards Coordinates operations #10314
  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
  • New functions/methods are listed in api.rst

cc @benbovy, @scottyhq

Copy link
Member

@benbovy benbovy left a comment

Choose a reason for hiding this comment

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

LGTM, those methods seem like a useful addition to me. Left a few minor comments.

@@ -561,6 +561,31 @@ def merge(self, other: Mapping[Any, Any] | None) -> Dataset:
variables=coords, coord_names=coord_names, indexes=indexes
)

def __or__(self, other: Self) -> Self:
"""Merge two sets of coordinates to create a new Coordinates object
Copy link
Member

Choose a reason for hiding this comment

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

Do we need such docstrings for an operator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not really, but it serves as a comment in this case. No need to copy everything, though, so we can remove everything but the first line and add a See Also section to Coordinates.merge

renamed : Coordinates
Coordinates object with renamed dimensions.
"""
return self.to_dataset().rename_dims(dims_dict, **dims).coords
Copy link
Member

Choose a reason for hiding this comment

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

self._data would have prevented duplicated copy of the wrapped Dataset, but it won't work with the DataArrayCoordinates subclass so I guess self.to_dataset() is fine.

keewis and others added 2 commits June 28, 2025 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants