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

Use standard_name mapper in rename_like #165

Closed
wants to merge 2 commits into from

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented Feb 4, 2021

Closes #134

@@ -214,8 +214,10 @@ def test_getitem_ancillary_variables():
def test_rename_like():
original = popds.copy(deep=True)

with pytest.raises(KeyError):
popds.cf.rename_like(airds)
# it'll match for axis: X (lon, nlon) and coordinate="longitude" (lon, TLONG)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We used to only check "coordinates", now we check all keys...

@@ -1312,18 +1310,21 @@ def rename_like(
ourkeys = self.keys()
theirkeys = other.cf.keys()

good_keys = set(_COORD_NAMES) & ourkeys & theirkeys
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we still raise an error?

popds.cf.rename_like(airds)
# it'll match for axis: X (lon, nlon) and coordinate="longitude" (lon, TLONG)
# so delete the axis attributes
newair = airds.copy(deep=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

would switching to fixtures for the datasets get around this?

theirs = apply_mapper(
(_get_axis_coord, _get_with_standard_name), other, key, error=False
)
if len(ours) > 1 or len(theirs) > 1:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could optionally raise a warning here if verbose=True

Copy link
Member

Choose a reason for hiding this comment

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

Maybe warn ant the end about all other.variables missing in renamer.values()?

Copy link
Member

Choose a reason for hiding this comment

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

This might work out with staggered grids: Rename axes first, if len(ours)>1 or len(theirs)>1 and all variables in ours and theirs have different combinations of dimensions we should be able to rename. As we already renamed the dimensions, we just have to look at their dims.

@dcherian dcherian mentioned this pull request Feb 20, 2021
4 tasks
malmans2 added a commit that referenced this pull request Feb 23, 2021
@dcherian dcherian closed this Feb 23, 2021
@dcherian dcherian deleted the better-rename branch February 23, 2021 17:59
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.

Match standard_name in .cf.rename_like
2 participants