Added rename_coords and rename_dims

[jukent]

• Closes #xxxx
• Tests added
• Fully documented, including whats-new.rst for all changes and api.rst for new API

[pep8speaks]

Hello @jukent! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file xarray/core/dataset.py:

Line 2217:9: F841 local variable 'coord_names' is assigned to but never used
Line 2313:5: E303 too many blank lines (2)
Line 2344:58: W291 trailing whitespace
Line 2349:5: E303 too many blank lines (2)
Line 2382:5: E303 too many blank lines (2)

• In the file xarray/tests/test_dataset.py:

Line 2115:32: E231 missing whitespace after ':'
Line 2115:37: E231 missing whitespace after ','
Line 2115:40: E231 missing whitespace after ','
Line 2115:42: E231 missing whitespace after ','
Line 2115:46: E231 missing whitespace after ','
Line 2115:50: E231 missing whitespace after ':'
Line 2115:55: E231 missing whitespace after ','
Line 2115:59: E231 missing whitespace after ','
Line 2115:62: E231 missing whitespace after ','
Line 2115:67: E231 missing whitespace after ','
Line 2115:71: E231 missing whitespace after ':'
Line 2116:32: E231 missing whitespace after ':'
Line 2116:41: E231 missing whitespace after ','
Line 2116:44: E231 missing whitespace after ','
Line 2116:46: E231 missing whitespace after ','
Line 2116:50: E231 missing whitespace after ','
Line 2116:54: E231 missing whitespace after ':'
Line 2116:63: E231 missing whitespace after ','
Line 2116:67: E231 missing whitespace after ','
Line 2116:70: E231 missing whitespace after ','
Line 2116:75: E231 missing whitespace after ','
Line 2116:79: E231 missing whitespace after ':'
Line 2116:80: E501 line too long (83 > 79 characters)
Line 2117:25: E231 missing whitespace after ':'
Line 2122:32: E231 missing whitespace after ':'
Line 2122:37: E231 missing whitespace after ','
Line 2122:40: E231 missing whitespace after ','
Line 2122:42: E231 missing whitespace after ','
Line 2122:46: E231 missing whitespace after ','
Line 2122:50: E231 missing whitespace after ':'
Line 2122:55: E231 missing whitespace after ','
Line 2122:59: E231 missing whitespace after ','
Line 2122:62: E231 missing whitespace after ','
Line 2122:67: E231 missing whitespace after ','
Line 2122:71: E231 missing whitespace after ':'
Line 2123:36: E231 missing whitespace after ':'
Line 2123:41: E231 missing whitespace after ','
Line 2123:44: E231 missing whitespace after ','
Line 2123:46: E231 missing whitespace after ','
Line 2123:50: E231 missing whitespace after ','
Line 2123:54: E231 missing whitespace after ':'
Line 2123:59: E231 missing whitespace after ','
Line 2123:63: E231 missing whitespace after ','
Line 2123:66: E231 missing whitespace after ','
Line 2123:71: E231 missing whitespace after ','
Line 2123:75: E231 missing whitespace after ':'
Line 2124:25: E231 missing whitespace after ':'

Comment last updated at 2019-06-25 20:30:26 UTC

[jukent]

#3026

There is some unexpected behavior. The new rename_dims function does not change the dimensions of the variables. Working on this now.

[jukent]

#3026

There is some unexpected behavior. The new rename_dims function does not change the dimensions of the variables. Working on this now.

Similarly rename_vars will rename the variable dimensions and variables, but not the dataset dimensions. I thought I was dealing with this with the _rename_indexes. But will have to keep working.

[max-sixty]

What do people think about vars vs variables here?

I'm +0.2 on variables for consistency with merge_variables etc (but only did a quick search, I would update if there were more public vars methods around)

[max-sixty]

Given inplace has been deprecated, I don't think we should add it here

[jukent]

Makes sense. I removed it, but it is still floating around in the other functions in the script (swap_dims for example)

[max-sixty]

This is great! Thanks @jukent ! Ping back here if there's anything we can do to help

[max-sixty]

And tbc, we should add tests here. That will help clarify the issues above re exactly which items on the dataset each method should rename vs leave.

[jukent]

And tbc, we should add tests here. That will help clarify the issues above re exactly which items on the dataset each method should rename vs leave.

Yes I need to add tests, but I don't have much experience with this yet. I will ask my team here to help me.

[jukent]

#3026
There is some unexpected behavior. The new rename_dims function does not change the dimensions of the variables. Working on this now.

Similarly rename_vars will rename the variable dimensions and variables, but not the dataset dimensions. I thought I was dealing with this with the _rename_indexes. But will have to keep working.

This works now! But to do so I created three different renaming variable helper functions (var name only, var and dims, var dims only). I will clean this up since the majority of these three functions is the same.

[max-sixty]

@jukent check out the existing rename tests here (they're a bit over-complicated at the beginning): https://github.com/pydata/xarray/blob/master/xarray/tests/test_dataset.py#L2046-L2130

You could do something as simple as this: https://github.com/pydata/xarray/blob/master/xarray/tests/test_dataset.py#L2114-L2119 - i.e. start with a dataset, call a rename method, and compare the dataset with a manually constructed version of the expected result.

Ping back with any issues

[jukent]

@jukent check out the existing rename tests here (they're a bit over-complicated at the beginning): https://github.com/pydata/xarray/blob/master/xarray/tests/test_dataset.py#L2046-L2130

You could do something as simple as this: https://github.com/pydata/xarray/blob/master/xarray/tests/test_dataset.py#L2114-L2119 - i.e. start with a dataset, call a rename method, and compare the dataset with a manually constructed version of the expected result.

Ping back with any issues

Thanks @max-sixty. I added some very simple tests. I might copy more of the test_rename functions (if I tried to rename a variable to the same name for example)

[max-sixty]

Those look great!

[max-sixty]

@jukent I don't think that's our bot