API: Index.take discards fill_value and raises unnecessarily

[jbrockmendel]

Index.take has two bugs when allow_fill=True and fill_value is passed explicitly:

1. fill_value is silently discarded

The caller's fill_value is ignored; self._na_value is always substituted:

# Note: we discard fill_value and use self._na_value, only relevant
#  in the case where allow_fill is True and fill_value is not None
taken = algos.take(
    values, indices, allow_fill=allow_fill, fill_value=self._na_value
)

2. _can_hold_na gate raises on integer dtypes

>>> pd.Index([1, 2, 3]).take([0, -1], allow_fill=True, fill_value=99)
ValueError: Unable to fill values because Index cannot contain NA

The caller provided a valid fill value that the dtype can hold, but Index.take rejects it because _can_hold_na is False for integer dtypes.

Consequences

These force internal callers to bypass Index.take and work on the underlying arrays directly:

• frame.py idxmin/idxmax: algorithms.take(index._values, ...) instead of index.take(...)
• reshape/merge.py: appends a sentinel value to the index, with a comment: "We do not use allow_fill and fill_value because it throws a ValueError on integer indices"
• indexes/base.py reset_index: take-then-putmask as two operations
• io/formats/excel.py: guards allow_fill on levels._can_hold_na
• indexes/multi.py: algos.take_nd(lev._values, ...) instead of lev.take(...)

Proposed fix

• Respect the caller's fill_value
• Remove the _can_hold_na gate (let the underlying array handle type promotion)
• Simplify the workaround sites

This is backward-compatible: the existing gate (allow_fill=True + fill_value is None → numpy-style take) is preserved, so bare idx.take(indices) is unchanged.

Separately, there are questions about the right long-term defaults for allow_fill (currently True for backward compat but arguably should be False to match ExtensionArray.take), but that would require a deprecation cycle and is out of scope here.

[jorisvandenbossche]

Sounds good to me

This is backward-compatible: the existing gate (allow_fill=True + fill_value is None → numpy-style take) is preserved, so bare idx.take(indices) is unchanged.

There is of course a change in behaviour when you pass a fill_value, but that's the bug fix I assume

Separately, there are questions about the right long-term defaults for allow_fill (currently True for backward compat but arguably should be False to match ExtensionArray.take), but that would require a deprecation cycle and is out of scope here.

While the default of Index.take says allow_fill=True, that seems a bit misleading as in practice the default is actually False anyway, since fill_value is not used by default, resulting in numpy-style behaviour.

So I think we should indeed match that default in the signature/docstring with the actual practice to avoid confusion. I suppose we only need to deprecate the case when someone is actually passing fill_value (otherwise allow_fill value has no effect anyway)?

[jorisvandenbossche]

defaults for allow_fill (currently True for backward compat

FWIW I am not sure how this could be about back compat (numpy's take doesn't even have it, so for back compat with the time Index was an ndarray subclass, it would be the other way around).
I would guess the reason for the default is the convenience to allow the user to to just specify a fill_value and already get the desired behaviour. While if allow_fill is False by default, then whenever you specify a fill_value, you also have to specify allow_fill=True as well to actually let the fill_value have any effect.