Support .sel with method kwarg for slices

[TomNicholas]

I feel like I must have missed some forbidden case here - it seemed too simple to effectively replace the use of the pandas.Index.slice_indexer method...

• Closes .sel with method doesn't work for slices #10710
• Tests added
• User visible changes (including notable bug fixes) are documented in whats-new.rst
• New functions/methods are listed in api.rst

[max-sixty]

this looks pretty good!!

do we need to be concerned about method as a dim we're selecting over?

[TomNicholas]

do we need to be concerned about method as a dim we're selecting over?

method is already in the API, I didn't touch that part, so I assume (hope) that is already handled at a higher-level somewhere.

[keewis]

either way, selecting from coordinates named method can still be done by passing a dict

[max-sixty]

method is already in the API, I didn't touch that part, so I assume (hope) that is already handled at a higher-level somewhere.

sorry, you're right!

[shoyer]

This omission was definitely back in the day!

I no longer recall exactly why, but I think the main concern was about potentially ambiguous behavior. In particular, default slicing works similar to method='backfill' for the left bound and method='pad' for the right bound, which isn't possible to express with a single method argument.

Thinking about this now, I think this is probably safe and would be a welcome new feature, though it may be worth considering supporting the "default" method=None with an explicit tolerance.

[shoyer]

You need to handle the "no match" case, for which get_indexer() returns -1.

[TomNicholas]

I handled this in d05ab1a, with the implementation now returning an empty slice if either endpoint is not found.

[shoyer]

What happens if the index is non-unique?

[TomNicholas]

The behaviour of the existing implementation is confusing. This passes:

data_non_unique = xr.Dataset(
    coords={"lat": ("lat", [20.1, 21.1, 21.1, 22.1, 22.1, 23.1])}
)
expected = xr.Dataset(coords={"lat": ("lat", [21.1, 21.1, 22.1, 22.1])})
actual = data_non_unique.sel(lat=slice(21.1, 22.1))
assert_identical(expected, actual)

but it relies upon calling pandas.Index.slice_indexer on a non-unique index, despite the docstring of that method saying "Index needs to be ordered and unique."!

Also, triggering my implementation (using actual = data_non_unique.sel(lat=slice(21.0, 22.2), method="nearest")) currently fails at .get_indexer with pandas.errors.InvalidIndexError: Reindexing only valid with uniquely valued Index objects. This restriction seems reasonable, but is not documented in the docstring of .get_indexer.

So to get the current (intuitive) behaviour we are relying on undefined behaviour in pandas, and we can't support method/tolerance for slices on non-unique indexers using .get_indexer because of undocumented restrictions in pandas 🙃

[TomNicholas]

At least I can clearly catch this case, which I've done in 199765e. That means this PR now reduces the NotImplementedError surface from "any Index passed a slice with method" to "any non-unique Index passed a slice with method".

[TomNicholas]

it may be worth considering supporting the "default" method=None with an explicit tolerance.

I added support for this in 8cce331, and a test that confirms that with a big enough value of tolerance this does reduce to the "default" behaviour, comparing against when tolerance is not supplied.