Raise exception in to_dataset if resulting variable is also the name of a coordinate

[mgunyho]

Let me know if you think the error message is unclear or too verbose or too fancy or something.

• Closes DataArray.to_dataset(dim) silently drops variable if it is already a dim #7823
• Tests added
• User visible changes (including notable bug fixes) are documented in whats-new.rst

[max-sixty]

This looks great, thanks @mgunyho .

I'll merge in the next day or so unless anyone has feedback.

Test failures are unrelated I think, will check they fal on main too

[max-sixty]

I think we need to adjust the error message tests for the tests to pass

[mgunyho]

Ah of course, I missed that! Fixed now.

[max-sixty]

Thanks @mgunyho !

Unfortunately we also have a break in the docs — would you care to take a look? https://readthedocs.org/projects/xray/builds/22540466/

(This also likely means the docs were bad, so it's good news your code found it!)

[mgunyho]

Huh, at a quick glance this seems like a problem with the error raising logic/my code rather than with the docs. I'll look into it tomorrow.

[mgunyho]

Yeah, the issue was indeed with my code, it didn't show up in the tests because test_to_dataset_split was using just a 1D array, and in that case the problem didn't show up. I've updated that test now and fixed the logic. (I also rebased onto main)

[mgunyho]

I can't tell if the tests fail because of my code or something else. I changed the logic from {k: v for k, v in self._coords.items() if k != dim} to {c: self._coords[c] for c in coord_names}, where coord_names is a set, so the order might differ. I thought that my version would be easier to understand. So either the tests are assuming a specific order for the coords, or the failures are due to something unrelated.

[max-sixty]

I can't tell if the tests fail because of my code or something else. I changed the logic from {k: v for k, v in self._coords.items() if k != dim} to {c: for c in self._coords[c] for c in coord_names}, where coord_names is a set, so the order might differ. I thought that my version would be easier to understand. So either the tests are assuming a specific order for the coords, or the failures are due to something unrelated.

To respond quickly though not fully — does the previous approach work? It may well require a specific order. If the former works well then that's fine...

Thanks for adding more tests, really good we caught the issue prior

[mgunyho]

I was also confused because I couldn't reproduce the error locally. But I changed it back to the original self._coords.items() version.

[mgunyho]

Is this too weird, should I try to think of another way to express this?

[mgunyho]

Hm, at least the failures in https://github.com/pydata/xarray/actions/runs/6869006192/job/18680869980?pr=8433 and https://github.com/pydata/xarray/actions/runs/6860572749/job/18654637893?pr=8433 seem to be the same for both implementations, so maybe it's not my code but something else.

[mgunyho]

Yep, here's the same failure from another PR: https://github.com/pydata/xarray/actions/runs/6865796013/job/18670620776?pr=8450

[max-sixty]

Super, thanks @mgunyho !

(Sorry for the test failures, hopefully those will resolve shortly...)