add drop_existing kwarg to set_xindex

[ianhi]

• Closes Add new kwarg drop_existing to set_xindex #11007
• Tests added
• User visible changes (including notable bug fixes) are documented in whats-new.rst
• [NA] New functions/methods are listed in api.rst

[benbovy]

Thanks @ianhi, I think that this is a convenient addition!

I'd just rename drop_existing to simply drop. The latter is a bit less self-explanatory but more consistent with the rest of the Xarray API where drop is already a parameter of many methods like .reset_index, .reset_coords, .isel, .where, etc.

[max-sixty]

just to confirm — and I left a comment at #11007, so possibly folks saw this — we're sure this shouldn't replace by default? currently the closest function is set_index, which does exactly that, iiuc

[benbovy]

we're sure this shouldn't replace by default? currently the closest function is set_index, which does exactly that, iiuc

I don't have strong opinions on this, we could replace it by default indeed. It might break the workflow of some users who rely on explicit-only index replacement as a safety guard, however to my knowledge people are actually rather annoyed by that (including me :-)).

[ianhi]

re the default I was defaulting to being cautious given xarray's large userbase. But I think it's in general a large user base, perhaps not for this specific function. I agree that a default of drop=True is preferable. So the quesiton becomes are we comfortable with just doing that, or does that require a deprecation cycle?

[benbovy]

So the quesiton becomes are we comfortable with just doing that

Yes I think we are!

[max-sixty]

(this might be going too far, but we may not even need the drop kwarg...)

[benbovy]

this might be going too far, but we may not even need the drop kwarg...

Agreed!

In most (in not all) Xarray API methods mentioned above with a drop parameter, it is about dropping coordinate variables. But that is not the case here.

The initial and only reason for not automatically dropping existing indexes in .set_xindex() was to prevent accidentally removing custom indexes that were expensive to build, like a big kd-tree. This may not be a very good reason, though. A user calling .set_xindex() has a pretty clear intent, which in 90% of the cases is to set or replace an index. For the 10% remaining cases a manual check if "coord_name" not in ds.xindexes is good enough!

[ianhi]

I have removed the kwarg entirely. this is now the default and only behavior.