Fix inconsistent behaviour for xarray.DataArray.rename #1178
Conversation
magonser
changed the title
DataArray.rename to a more explicit behavior.
| names to new names for coordinates. Otherwise, use the argument | ||
| as the new name for this array. | ||
|
|
||
| new_dims_dict : dict-like |
There was a problem hiding this comment.
Instead of adding a second argument here, I would simply require calling .rename() twice if you want to rename both the DataArray and it's coordinates at the same time.
There was a problem hiding this comment.
I've been thinking about that initially, too. Fixed in f855063.
shoyer
changed the title
| @@ -788,16 +788,17 @@ def reindex(self, method=None, tolerance=None, copy=True, **indexers): | |||
| method=method, tolerance=tolerance, copy=copy, **indexers) | |||
| return self._from_temp_dataset(ds) | |||
|
|
|||
| def rename(self, new_name_or_name_dict): | |||
| """Returns a new DataArray with renamed coordinates and/or a new name. | |||
| def rename(self, new_name_or_dims_dict): | |||
There was a problem hiding this comment.
Let's keep this parameter name the same as before.
| @@ -811,12 +812,8 @@ def rename(self, new_name_or_name_dict): | |||
| """ | |||
| if utils.is_dict_like(new_name_or_name_dict): | |||
| name_dict = new_name_or_name_dict.copy() | |||
There was a problem hiding this comment.
It occurs to me that the copying here is no longer necessary, since we no longer modify the dictionary.
There was a problem hiding this comment.
I have pushed another commit fa6b02c to remove it. Keeping the PR open for another while is fine by me.
|
Slightly OT, but it occurred to me that we took the habit not to add an attribution (e.g. "By bla") to the "Breaking changes" list. This is probably nicer for the people who are quickly reading through the changes, but not really consistent with the bugfixes or enhancements. In that case, I think you could list your change in the "bugfix" list rather than "breaking changes" |
|
@fmaussion that was my mistake here -- I didn't give myself credit on a few of the "Breaking Changes" but we should fix that. @magonser want to insert a line giving yourself credit for this change? |
|
Alright, I've added attribution. Thanks by the way for being patient with me in this PR, as I am just getting started on open source contributions. |
|
Thanks @magonser, we really appreciate PRs, especially from new contributors! |
Attempt to fix #1177
Please let me know, if PR requires further refinement.