BUG: Fix Index.droplevel() stub — use Sequence[Never] to allow only droplevel([]) (GH#1678)#1732
Conversation
…alid (GH#1678) Index.droplevel() always raises ValueError at runtime because a plain Index has exactly one level. Only MultiIndex.droplevel() is meaningful. Remove droplevel() from the Index base stub so type checkers (pyright/mypy) report an error when users call it on a plain Index, prompting them to use cast(MultiIndex, idx).droplevel(...) explicitly. - Remove droplevel() from pandas-stubs/core/indexes/base.pyi - Drop now-unnecessary # type: ignore[override] comment on MultiIndex.droplevel() - Update test: move Index.droplevel() to TYPE_CHECKING_INVALID_USAGE block
loicdiridollou
left a comment
loicdiridollou
left a comment
There was a problem hiding this comment.
pd.Index().droplevel([]) does not raise an error and I believe should be kept, @Dr-Irv what do you think?
Well, I'm the one that created the issue, and that means it happened in code my team was working on, so I'm fine with having the type checker say that |
|
Sorry not clear, my argument was that |
From what I've read, type checkers are not able to distinguish between empty sequences and non-empty sequences. But if you can get it to work, go ahead. |
… while rejecting all other args (GH#1678)
droplevel() on a plain Index always raises ValueError unless called with an
empty sequence. The previous fix (deleting the stub entirely) was overly strict
because droplevel([]) is a documented no-op that works at runtime.
Use Sequence[Never] instead — type checkers (mypy and pyright both verified)
correctly allow droplevel([]) while rejecting droplevel(0), droplevel([0]),
droplevel('name'), etc.
The type: ignore[override] on MultiIndex.droplevel stays — it is needed because
the return type MultiIndex | Index is wider than Self (what the parent stub
declares), not because of argument types.
tinezivic
changed the title
|
Hey @loicdiridollou — you were right, I tested it and I updated the PR: The |
|
Done, removed all comments. |
loicdiridollou
left a comment
loicdiridollou
left a comment
There was a problem hiding this comment.
On minor thing, rest looks good, check CI I kicked it to make sure it passes or not
Co-authored-by: Yi-Fan Wang <cmp0xff@users.noreply.github.com>
Co-authored-by: Yi-Fan Wang <cmp0xff@users.noreply.github.com>
cmp0xff
left a comment
cmp0xff
left a comment
There was a problem hiding this comment.
Looks good to me. I would not move droplevel from L434 to L454, but it's not a bit deal.
I'll hand over to @loicdiridollou to merge.
loicdiridollou
left a comment
loicdiridollou
left a comment
There was a problem hiding this comment.
All good, thanks for your contribution @tinezivic !
4 participants
Index.droplevel()should not be permitted #1678assert_type()to assert the type of any return value)AGENTS.md.Problem
Index.droplevel()was defined in theIndexbase stub, but calling it on a plainIndexwith any non-empty argument always raisesValueErrorat runtime. A plainIndexhas exactly one level — you cannot remove it.Type checkers accepted
idx.droplevel(0)without complaint, misleading users into thinking this call is safe.Fix
Use
Sequence[Never]as the parameter type forIndex.droplevel():Sequence[Never]means "a sequence with no possible elements" — in practice, only the empty list[]satisfies this type. Both mypy and pyright (verified with actual tests) correctly:idx.droplevel([])— which is a documented no-op at runtimeidx.droplevel(),idx.droplevel(0),idx.droplevel([0]),idx.droplevel("name")— all of which raiseValueErrorThis is more precise than deleting the method entirely, because
droplevel([])is documented behavior.Changes
base.pyi: adddroplevel(level: Sequence[Never]) -> Selfmulti.pyi:droplevelunchanged;# type: ignore[override]stays because the return typeMultiIndex | Indexis wider thanSelf(needed for correctness, unrelated to our change)tests/indexes/test_indexes.py: addidx.droplevel([])as a valid usage; expandTYPE_CHECKING_INVALID_USAGEblock with four invalid cases (droplevel(),droplevel(0),droplevel([0]),droplevel("name"))Why Sequence[Never] works
Both mypy and pyright treat
[]as assignable toSequence[Never](an empty sequence has no elements, so the element type constraint is vacuously satisfied), but reject any non-empty sequence. This is the most accurate annotation for the actual runtime contract on a plainIndex.