Skip to content

Unpacking an iterable with an __iter__ method that returns self leads to variable being silently inferred as Any #14819

Closed

Description

@AlexWaygood

This is similar to #14811, but still reproduces on master following bed49ab:

from typing import TypeVar

T = TypeVar("T")

class Foo:
    count: int
    def __init__(self) -> None:
        self.count = 0
    def __iter__(self: T) -> T:
        return self
    def __next__(self) -> int:
        self.count += 1
        if self.count > 3:
            raise StopIteration
        return self.count

a, b, c = Foo()
reveal_type(a)  # note: Revealed type is "Any"

Mypy should have enough information here to infer that the type of a is int.

Mypy versions tested on

Activity

  1. AlexWaygood commented on Mar 2, 2023

    @AlexWaygood
    MemberAuthor

    This one-line change fixes the issue:

    --- a/mypy/checker.py
    +++ b/mypy/checker.py
    @@ -3633,7 +3633,7 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
         ) -> None:
             rvalue_type = get_proper_type(rvalue_type)
             if self.type_is_iterable(rvalue_type) and isinstance(rvalue_type, Instance):
    -            item_type = self.iterable_item_type(rvalue_type)
    +            _, item_type = self.analyze_iterable_item_type_without_expression(rvalue_type, context)

    But it causes this test to fail:

    [case testInferringTypesFromIterable]
    from typing import Iterable
    class Nums(Iterable[int]):
    def __iter__(self): pass
    def __next__(self): pass
    a, b = Nums()
    if int():
    a = b = 1
    if int():
    a = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int")
    if int():
    b = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int")
    [builtins fixtures/for.pyi]

    The test succeeds once again if we add annotations:

    --- a/test-data/unit/check-inference.test
    +++ b/test-data/unit/check-inference.test
    @@ -375,10 +375,10 @@ def f(d: Any) -> None:
         b.x
    
     [case testInferringTypesFromIterable]
    -from typing import Iterable
    +from typing import Iterable, Iterator
     class Nums(Iterable[int]):
    -    def __iter__(self): pass
    -    def __next__(self): pass
    +    def __iter__(self) -> Iterator[int]: pass
    +    def __next__(self) -> int: pass

    @sobolevn, do you think this would be a reasonable fix? It makes me wonder why we have the iterable_item_type() method at all, and whether other uses of this method in checker.py should also be converted to analyze_iterable_item_type_without_expression()

  2. AlexWaygood commented on Mar 2, 2023

    @AlexWaygood
    MemberAuthor

    I can't tell whether the required change to testInferringTypesFromIterable would be a feature or a bug. Mypy usually infers that unannotated methods return Any. It feels weird to me that this test case is currently asserting that mypy should infer a non-Any type here, despite both __iter__ and __next__ being unannotated.

  3. sobolevn commented on Mar 2, 2023

    @sobolevn
    Member

    It makes me wonder why we have the iterable_item_type() method at all, and whether other uses of this method in checker.py should also be converted to analyze_iterable_item_type_without_expression()

    I don't really know. It was my first Iterable-related PR.

    I can't tell whether the required change to testInferringTypesFromIterable would be a feature or a bug

    It is really hard to tell whether your solution is a bug or an improvement (sadly, python's type system is not documented enough).

  4. AlexWaygood commented on Mar 2, 2023

    @AlexWaygood
    MemberAuthor

    Thanks! I guess I'll experiment with a broader refactor and see if anything (else) breaks :)

  5. self-assigned this
    on Mar 2, 2023
  6. AlexWaygood commented on Mar 2, 2023

    @AlexWaygood
    MemberAuthor

    The test was introduced in 2013, in 6ecbd1f

  7. AlexWaygood commented on Mar 2, 2023

    @AlexWaygood
    MemberAuthor

    There is another test in a different file that asserts the same behaviour, so I'm going for a slightly different approach:

    [case testMultipleAssignmentAndGenericSubtyping]
    from typing import Iterable
    n, s = None, None # type: int, str
    class Nums(Iterable[int]):
    def __iter__(self): pass
    def __next__(self): pass
    n, n = Nums()
    s, s = Nums() # E: Incompatible types in assignment (expression has type "int", variable has type "str")
    [builtins fixtures/for.pyi]
    [out]

  8. sterliakov commented on Mar 2, 2023

    @sterliakov
    Collaborator

    Is the behaviour in those two tests really expected? As per docs, all untyped methods should be treated as annotated with Any for all arguments and return type, while here this rule is violated (__iter__ is unannotated, so it's essentialy unpacking Any, which should give n-tuple of Anys of size corresponding to LHS). The inherited signature should not be taken into account. The proposed change fixes this behaviour and makes handling of such cases more consistent - what's the reasoning behind that tests?

  9. AlexWaygood commented on Mar 2, 2023

    @AlexWaygood
    MemberAuthor

    @sterliakov, I don't disagree with anything you're saying. I'm just somewhat hesitant to make that change here, given that these tests have been in place for 10 years or more -- it could be the case that many people are depending on this being established mypy behaviour. The fact that two tests assert the same behaviour indicates that it was a deliberate decision, even if we might disagree with that decision now.

    Fixing the original bug in this issue thread while maintaining the behaviour asserted in these two tests actually appears to be fairly trivial, so for now, I'm planning on opening a PR to do that. I'll open a followup issue after that to discuss whether the behaviour that's asserted in these two tests should be changed.

  10. added a commit that references this issue on Mar 3, 2023
    456dcbd
  11. AlexWaygood commented on Mar 3, 2023

    @AlexWaygood
    MemberAuthor

    @sterliakov, the behaviour does appear to be consistent with some other parts of mypy also. For example:

    from typing import Iterable
    
    class Foo(Iterable[int]):
        def __iter__(self):
            yield from range(5)
    
    x = list(Foo())
    reveal_type(x)  # Revealed type is "builtins.list[builtins.int]"

    https://mypy-play.net/?mypy=latest&python=3.11&gist=689e0a0950b2f48cf9608ffe03c47926

    I guess that this is a general principle applied by mypy that I wasn't aware of, where classes that directly inherit from protocols are assumed to have the more specific signature of their superclass, even if the method on the subclass is unannotated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions