Add transpose_coords option to DataArray.transpose

[phausamann]

• Closes Option to make DataArray.transpose also transpose coords #1856
• Tests added (for all bug fixes or enhancements)
• Fully documented, including whats-new.rst for all changes and api.rst for new API

I've added the option transpose_coords to DataArray.transpose as described in #1856. It's False by default, as it breaks a couple of tests otherwise (TestConcatDataset.test_concat, test_apply_output_core_dimension and test_dot). I'm not sure whether fixing these tests to work with the new behavior should be part of the PR.

[pep8speaks]

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

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-05-21 16:42:27 UTC

[shoyer]

As a default, I think we want transpose_coords=None.

If transpose_coords is None and there are coordinates to transpose, we can issue a FutureWarning about how the default behavior of transpose_coords will switch to True in the future.

I'm not sure whether fixing these tests to work with the new behavior should be part of the PR.

Yes, this would be ideal. It would be fine to use transpose_coords=False explicitly in tests, but we should silence all warnings.

[phausamann]

I've updated the method to issue the warning and fixed the failing tests (which was actually a bug in my implementation).

It would be fine to use transpose_coords=False explicitly in tests, but we should silence all warnings.

I'm not exactly sure how this works, there are a lot of tests where the warning is issued, especially calls to DataArray.T. Should I update all of these tests to transpose(..., transpose_coords=False)?

[shoyer]

I'm not exactly sure how this works, there are a lot of tests where the warning is issued, especially calls to DataArray.T. Should I update all of these tests to transpose(..., transpose_coords=False)?

Yes, that would probably be the right call. Or maybe even switch to .transpose(transpose_coords=True) to future proof the behavior?

We try to avoid issuing warnings in xarray's own test suite so we notice issues (e.g., deprecations in upstream packages) when they arise. This also gives us some estimate of how much turmoil a change will cause for users -- if we end up with lots of internal warnings, then users will probably notice, too.

[phausamann]

I've changed all the transpose calls in the tests to transpose_coords=True. Other than that, there's some transpose calls in plot and groupby that I've updated, but I'm not sure whether they should be True, False or not updated at all (see above).

[phausamann]

I've added a restore_coord_dims parameter to groupby, groupby_bins and resample and eliminated all warnings from the tests. The build failure seems to be inherited from the master branch.

[phausamann]

@shoyer is this ready to be merged?

[StanczakDominik]

Hi, just popping in - perhaps rebasing on top of the master branch would help move this along?

[shoyer]

I just merged in master, let's see how that goes

[phausamann]

Looks like the build failures were only temporary, all the checks are passing now. I think this is good to go.

[shoyer]

OK, in it goes!

Thanks for your patience @phausamann !