Fix issue tuple as dimension

[thomashirtz]

I edited dataset in core to try to solve the issue 4821. Some tests are still currently missing, also I believe there is still one more issue related to using tuple as dim.

The code xr.Dataset({"a": ([("a", "b")], [1])}) does not error yet.

• Closes #4821
• Tests added
• Passes pre-commit run --all-files
• User visible changes (including notable bug fixes) are documented in whats-new.rst

[max-sixty]

This looks good!

The one corner case I'm not sure this handles is when there is actually a tuple dimension. If that's correct, could we add a test for that case, and maybe here check that it's a tuple and it's not in the dimensions?

The code xr.Dataset({"a": ([("a", "b")], [1])}) does not error yet.

Great, tbc we want that construction to pass — we're saying it's allowable to have tuples as dimension names (while not encouraging it!)

Thanks @thomashirtz

[max-sixty]

This was quite close @thomashirtz — let me know if you'd be up for finishing it off. Thanks

[thomashirtz]

I would like to continue to contribute to xarray, unfortunately I have many important matters theses days. I may contribute again from end of summer. If someone wants to tackle this issue until then, he can

[headtr1ck]

I think this should be also aligned with str | Iterable[Hashable], so maybe str | DataArray | Iterable[Hashable | DataArray]?
And then reverse the isinstance checks (do not capture tuple and list explicitly, but str).

[max-sixty]

I think this should be also aligned with str | Iterable[Hashable], so maybe str | DataArray | Iterable[Hashable | DataArray]?

Good point...

IIUC (and it's been a while since I thought through this properly), this would support allowing tuples as dimension names, but forcing them to be passed within an iterable to methods (e.g.method(dim=[('a','b')]) for a dimension named ('a', 'b'). So we'd keep str | Iterable[Hashable], and still raise an error if a tuple was passed directly (i.e. method(dim=('a','b'))

[headtr1ck]

So we'd keep str | Iterable[Hashable], and still raise an error if a tuple was passed directly (i.e. method(dim=('a','b'))

I would say that a tuple is fine, it IS an iterable of hashable ;)

[max-sixty]

I would say that a tuple is fine, it IS an iterable of hashable ;)

Sorry, yes, I was unclear above!

How should we handle dimensions which are themselves tuples? (i.e. a really unlikely corner-case, but one that IIRC inspired some of the discussion in #4821)

My thought was to interpret a tuple as an iterable of dimensions. If we have a dimension name of a tuple, then that requires passing within an Iterable — i.e. the method(dim=[('a','b')]). Since the mypy changes around str | Iterable[Hashable], this seems better than the options I enumerated at #4821 (comment).

Does that make sense? V possible my writing and / or thinking is still not clear!

[headtr1ck]

Yes exactly, that would be the intended way of working with dimensions/variables that are iterablea themself, like tuples.