MNT: Fix mypy job for pytest 9.1.0

[MarcoGorelli]

closes #34292, looks like it's the classic covariance vs invariance of tuples vs lists

Reference Issues/PRs

What does this implement/fix? Explain your changes.

First time contributor introduction

AI usage disclosure

I used AI assistance for:

• Code generation (e.g., when writing an implementation or fixing a bug)
• Test/benchmark generation
• Documentation (including examples)
• Research and understanding

Any other comments?

[MarcoGorelli]

### Running mypy ###

Success: no issues found in 596 source files
No problem detected by mypy

nice 😎

[ogrisel]

Thanks @MarcoGorelli. Reading the error message for that particular line (chosen arbitrarily), I do not understand how you came up with that "fix":

sklearn/neighbors/tests/test_graph.py:84: error: List item 0 has incompatible type "type[KNeighborsTransformer]"; expected "type[NeighborsBase]"  [list-item]

According to the mypy message, the problem is the type of the item (KNeighborsTransformer vs NeighborsBase), not the type of the container (list vs tuple).

[MarcoGorelli]

hey - you can reproduce this more simply (and independently of pytest) with just

from typing import reveal_type

class Animal:
    ...

class Cat(Animal):
    def __init__(self, *, name: str) -> None:
        self._name = name

class Dog(Animal):
    def __init__(self, *, nickname: str) -> None:
        self._nickname = nickname

def main() -> None:
    _a = [Cat, Dog]
    reveal_type(_a)

t.py:15: error: List item 0 has incompatible type "type[Cat]"; expected "type[Animal]"  [list-item]
t.py:16: note: Revealed type is "list[def (*, nickname: str) -> t.Animal]"
Found 1 error in 1 file (checked 1 source file)

Mypy infers _a to be list[type[Animal]]. But then, checking each element:

• type[Cat] isn't assignable to type[Animal] because it has an incompatible constructor. If I expect a: type[Animal] and then call a(), I expect it to run, whereas if a was type[Cat], it wouldn't
• same story for type[Dog]

By using a tuple, on the other hand, it can just infer it to be

tuple[type[Cat], type[Animal]]

and, not being mutable, there's no worry that some other unexpected or incompatible type is going to make it into the sequence

---

I do not understand how you came up with that "fix":

😄 not sure if i'm misreading this, but do the double-quotes suggest you don't consider this to be the right fix?

[ogrisel]

not sure if i'm misreading this, but do the double-quotes suggest you don't consider this to be the right fix?

It might be. It's just that I do not understand the implications of mutability and found the mypy error message confusing.

I find it a sad that we have to change working code to work around a side effect of the type checker.

The official pytest documentation for @pytest.mark.parametrize still uses mutable lists so the scikit-learn code seems valid to me:

https://docs.pytest.org/en/stable/how-to/parametrize.html

[MarcoGorelli]

The official pytest documentation for @pytest.mark.parametrize still uses mutable lists so the scikit-learn code seems valid to me:

It's valid if the elements of the list are assignable to a list of their common type

If you have types of classes with incompatible constructors, like RadiusNeighborsTransformer and KNeighborsTransformer, then you'll need a tuple

If in the example I gave, if the constructors were

class Cat(Animal):
    def __init__(self, *, name: str) -> None:
        self._name = name

class Dog(Animal):
    def __init__(self, *, name: str) -> None:
        self._name = name

then mypy would accept it

Note also that this seems to be mypy-specific, pyright / pyrefly / ty are fine with it because they'd infer the list to be list[type[Cat] | type[Dog]] (I'll refrain from discussing whether scikit-learn should migrate to a different type checker in this discussion)

[ogrisel]

Alright thanks for the clarifications.

(I'll refrain from discussing whether scikit-learn should migrate to a different type checker in this discussion)

That case would be a valid reason to switch to a checker with more intuitive type inference semantics (or more explicit error messages). However, I suspect that other alternatives will push us to make other kinds of unwanted/unanticipated code changes.

[ogrisel]

After reading the detailed clarifications in the discussion above, this change looks good to me (and I see no alternative beyond switching to another type checker).

[MarcoGorelli]

thank you for your review!

[AnneBeyer]

Thank you for catching this and the helpful explanation, @MarcoGorelli!

[virchan]

LGTM! Thanks, @MarcoGorelli!

Thanks to @ogrisel and @AnneBeyer for the reviews.