issue_comments

Yes, we want to replace that test with your test — either in the same test function or a new test function.

Though it maybe it should still be a KeyError, I think probably it should be. But feel free to submit the PR and it's easiest to review there.

Thanks!

If you allow explicitly tuple, doesn't it mean I need to edit this part

Yes!

(I find it a little bit weird to give dimensions in list, no ? I thought that generally tuples were more adapted to provide dims)

Yes, I feel similarly. For one xr.Dataset(dict(var=(('a','b'), np.random.rand(3)))) is an error, rather than a one-dimensioned dataset with a single dim called ('a','b'). But OTOH I can very much empathize with accepting any Hashable for dim names most functions, which includes tuples. So I am fine formalizing this (apart from the constructor).

Can you tell me which test-case I can implement to fulfill all the requirements needed ? I can't think of test cases that I can implement except the one suggested first, and that one passes with the first modification

How about a test that checks the example above suggests a list, rather than raising a vanilla KeyError?

Cheers @thomashirtz

Great, agree @dcherian . So maybe we: - Check whether the dim exists (there could be a dim with tuple), which is already done in 1371 below - If it doesn't exist, then check whether it's a string before passing it to _get_virtual_variable below. If it's a tuple, that message can suggest passing it as a list. - Or we do the check in _get_virtual_variable?

python 1366 def _construct_dataarray(self, name: Hashable) -> "DataArray": 1367 """Construct a DataArray by indexing this dataset""" 1368 from .dataarray import DataArray 1369 1370 try: 1371 variable = self._variables[name] 1372 except KeyError: -> 1373 _, name, variable = _get_virtual_variable( 1374 self._variables, name, self._level_coords, self.dims 1375 )

@thomashirtz changing the labels back! Thanks in advance!

Any thoughts from others on what we should do, of the three options above? CC @pydata/xarray

I would propose we either: - Explicitly allow tuples as dimension and variable names. This is probably the most mypy-compatible, since we use Hashable in lots of places - Force dimension names to be strings, but allow variable names to be Hashable. This might be the most practical. - Disallow tuples for dimension names, but otherwise allow Hashable. Then tuples work the same as lists when supplied as arguments.