-
Notifications
You must be signed in to change notification settings - Fork 42
rename_like warns about conflicting variables #183
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
Conversation
cf_xarray/accessor.py
Outdated
@@ -1346,6 +1346,19 @@ def rename_like( | |||
theirs = _single(_get_all)(other, key)[0] | |||
renamer[ours] = theirs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there weird cases where this is actually a conflict and we should skip and warn about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no idea ;) lets find out.
Codecov Report
@@ Coverage Diff @@
## main #183 +/- ##
==========================================
+ Coverage 96.12% 96.13% +0.01%
==========================================
Files 11 11
Lines 1366 1372 +6
==========================================
+ Hits 1313 1319 +6
Misses 53 53
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Mattia.
It'd be nice if there was a way for the user to limit the renaming in some form but maybe this should be a future PR
pop.cf["TEMP"].cf.rename_like(ds, do=["coordinates", "standard_name"])
This would work, since it would skip nlon
, nlat
so there would be no conflicts.
Likewise
pop.cf["TEMP"].cf.rename_like(ds, do=["axes"])
woudl rename nlon
, nlat
to lon
, lat
cf_xarray/accessor.py
Outdated
@@ -1346,6 +1346,19 @@ def rename_like( | |||
theirs = _single(_get_all)(other, key)[0] | |||
renamer[ours] = theirs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no idea ;) lets find out.
cf_xarray/accessor.py
Outdated
@@ -1340,12 +1340,36 @@ def rename_like( | |||
theirkeys = other.cf.keys() | |||
|
|||
good_keys = ourkeys & theirkeys | |||
renamer = {} | |||
keydict = {} | |||
for key in good_keys: | |||
ours = _single(_get_all)(self._obj, key)[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we catch the error from _single
and just skip+warn?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes that sounds right
Looks like this now: from cf_xarray.datasets import airds, popds
popds.cf.rename_like(airds)
|
Thanks @malmans2 |
Closes #182