sel fails confusingly or silently when a dimension name matches an optional argument

[gwgundersen]

Given that many Xarray methods accept dict or keyword arguments, this may effect other methods. That said, here is a minimal example with sel.

Minimal example

If I want to select based on a dimension named method, I cannot because Xarray thinks method is the method argument to sel:

>>> da1 = xr.DataArray(range(3), dims=['method'], coords={'method': range(3)})
>>> da1
<xarray.DataArray (method: 3)>
array([0, 1, 2])
Coordinates:
  * method   (method) int64 0 1 2
>>> da1.sel(method=0)
...
TypeError: ``method`` must be a string

And if the method dimension has string labels, this fails silently:

>>> da2 = xr.DataArray(range(3), dims=['method'], coords={'method': list('abc')})
>>> da2.sel(method='a')
<xarray.DataArray (method: 3)>
array([0, 1, 2])
Coordinates:
  * method   (method) <U1 'a' 'b' 'c'

Expected Output

I think raising a ValueError and providing a clarifying error message is the right call, but maybe the maintainers have a different opinion. At the very least, it seems like Xarray could note that the DataArray instance has a dimension that matches one of the function arguments and ask the user to use dict-like arguments if required.

I imagine a general error handling function could be written to check this for any function and DataArray pair.

[max-sixty]

Yes, agree this isn't ideal. One of the great qualities of xarray is the code we write in exploration also works in production, and this behavior violates some of that.
I'd vote for a warning as getting some of the way to a solution

FWIW passing a dict to these methods will work, if you want complete dependability

[gwgundersen]

What if I create a PR with a function in utils that takes an array and a list of keyword arguments, and warns the user if the array has any of the keyword arguments as dimensions. The warning can suggest using a dict.

[dcherian]

+1. Though I prefer raising a more informative over raising a warning.

[max-sixty]

+1. Though I prefer raising a more informative over raising a warning.

Raising an error?

[dcherian]

Yes. fat fingers on mobile :)

[max-sixty]

Yes I agree, on reflection

[shoyer]

What if I create a PR with a function in utils that takes an array and a list of keyword arguments, and warns the user if the array has any of the keyword arguments as dimensions. The warning can suggest using a dict.

This sounds great to me!

I have also been bitten by this issue, particularly with the same dimension name method.

[shoyer]

This could be an addition to the either_dict_or_kwargs helper function:

xarray/xarray/core/utils.py

Line 248 in df25933

def either_dict_or_kwargs(

[gwgundersen]

This could be an addition to the either_dict_or_kwargs helper function.

Currently, either_dict_or_kwargs doesn't have access to the DataArray instance's dims or the function's keyword arguments; are you suggesting that either_dict_or_kwargs be modified to take these? I do like this approach. Otherwise, I'd have to add a second utility function check everywhere either_dict_or_kwargs is used, and it'd be easy to forget going forward.

[dcherian]

Yes add another kwarg