Allow selecting variables using a list with mixed data types #5394
Conversation
malmans2
changed the title
xarray/core/dataset.py
Outdated
| @@ -1485,7 +1485,7 @@ def __getitem__(self, key): | |||
| if hashable(key): | |||
| return self._construct_dataarray(key) | |||
| else: | |||
| return self._copy_listed(np.asarray(key)) | |||
| return self._copy_listed(np.asarray(key, dtype="O")) | |||
There was a problem hiding this comment.
Why is np.asarray needed here?
Can we just check that key is a list, and raise an error otherwise.
Right now, if key is for example a set (i.e., ds[set(["foo"])]), an error is raised anyways because np.asarray creates a 0-d array and iteration is not possible. The error is not raised directly by xarray though and I think it's not very clear, especially because the original set is iterable.
Also, is the goal to only allow lists? Does it make sense to allow any iterable?
There was a problem hiding this comment.
Line 1345 in 5d367db
Seems to only be used to loop through in
._copy_listed. I can't really think of a reason why it has to be forced to ndarray? names is also typed as an Iterable[Hashable], so I suppose it makes sense to allow any iterable.
There was a problem hiding this comment.
Does anything break if we remove it?
There was a problem hiding this comment.
All tests look OK removing np.asarray
There was a problem hiding this comment.
Great! Without wanting to ignore Chesterton's fence — let's do it unless we hear objections in the next day or so.
|
@malmans2 feel free to add a whatsnew in another PR! |
|
...and thank you for this contribution! |
pre-commit run --all-fileswhats-new.rstLists passed to
__getitem__are converted to arrays.np.asarrayinfers the dtype, and for example converts a list with str and int to an array of str. Forcingdtype="O"fixes the issue.