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

Preserve Coordinates when Indexing UxDataArray #1003

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

Conversation

philipc2
Copy link
Member

@philipc2 philipc2 commented Oct 9, 2024

Closes #945

Overview

  • Preserves coordinates when indexing a UxDataArray

@philipc2 philipc2 changed the title Preserve Coordinates when Indexing Preserve Coordinates when Indexing UxDataArray Oct 9, 2024
@philipc2 philipc2 requested a review from ahijevyc October 9, 2024 16:42
@philipc2 philipc2 self-assigned this Oct 9, 2024
Copy link
Collaborator

@ahijevyc ahijevyc 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. I tested the change and tried to replicate the issue I had #818 with the uxarray.UxDataArray.Indexes disappearing but had no problems.

However, given this fix, I can think of two other places a similar change might be needed:

coords=source_uxda.coords,

and

coords=source_uxda.coords,

@philipc2
Copy link
Member Author

philipc2 commented Oct 9, 2024

Looks good. I tested the change and tried to replicate the issue #818 with the uxarray.UxDataArray.Indexes disappearing but had no problems.

However, given this fix, I can think of two other places a similar change might be needed:

coords=source_uxda.coords,

and

coords=source_uxda.coords,

Good catches, I'll update these also.

@philipc2
Copy link
Member Author

philipc2 commented Oct 9, 2024

@ahijevyc

When remapping, should the coordinates from the original data array be dropped? It wouldn't make much sense to preserve them (unless I'm understanding it wrong), since the coordinates from the original array are no longer representative of the new remapped grid.

Copy link
Collaborator

@ahijevyc ahijevyc left a comment

Choose a reason for hiding this comment

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

The spatial coordinates should not be transferred, but if there are non-spatial coordinates like time, ensemble_member, or forecast_hour, then those should be transferred to the destination. I think that's why I originally thought the coords argument was necessary.

@philipc2
Copy link
Member Author

philipc2 commented Oct 9, 2024

The spatial coordinates should not be transferred, but if there are non-spatial coordinates like time, ensemble_member, or forecast_hour, then those should be transferred to the destination. I think that's why I originally thought the coords argument was necessary.

That makes sense! I wasn't considering the non-spatial cases.

Copy link
Collaborator

@ahijevyc ahijevyc left a comment

Choose a reason for hiding this comment

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

I provided a way to drop the remapped coordinate (which I agree, should not get transferred from the source) and still preserve any non-spatial coordinates.

@@ -148,7 +148,6 @@ def _inverse_distance_weighted_remap_uxda(
uxda_remap = uxarray.core.dataarray.UxDataArray(
data=destination_data,
name=source_uxda.name,
coords=source_uxda.coords,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@philipc2 Here's my solution to preserve any non-spatial coordinates (like time or ensemble_member) while dropping the spatial one:

Copy source_uxda.coords to a new xarray.Coordinates object. Then delete the key associated with the spatial dimension. Then use that new object as the coords argument to uxarray.core.dataarray.UxDataArray.

    destination_coords = source_uxda.coords
    del(destination_coords[destination_dim])

    # construct data array for remapping variable
    uxda_remap = uxarray.core.dataarray.UxDataArray(
        data=destination_data,
        name=source_uxda.name,
        coords=destination_coords,

Does this make sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

_slice_from_grid may need to use coords from a sliced dataarray
3 participants