Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change in inference of overloads involving Hashable and slice now that slice is generic #18149

Open
AlexWaygood opened this issue Nov 12, 2024 · 4 comments
Labels
bug mypy got something wrong

Comments

@AlexWaygood
Copy link
Member

Bug Report

Consider the following Python snippet, saved as foo.pyi:

from typing import assert_type, overload, Hashable

class Foo: ...

class DataFrame:
    @overload
    def __getitem__(self, key: slice) -> DataFrame: ...
    @overload
    def __getitem__(self, key: Hashable) -> Foo: ...


df = DataFrame()
assert_type(df[1:], DataFrame)
assert_type(df[:2], DataFrame)

Prior to the recent change to make slice generic in typeshed, mypy used to emit no errors on this snippet. However, mypy on the master branch emits the following errors:

foo.pyi:7: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [overload-overlap]
foo.pyi:13: error: Expression is of type "Any", not "DataFrame"  [assert-type]
foo.pyi:14: error: Expression is of type "Any", not "DataFrame"  [assert-type]

The first error here seems reasonable to me, but it's unclear to me why mypy now infers Any as the result of the df subscriptions. This snippet is minimized from the sole new mypy_primer error reported in python/typeshed#11637 (comment).

To Reproduce

https://mypy-play.net/?mypy=master&python=3.12&gist=339ba7f72c9048770ce15d6dc75207a1

Your Environment

  • Mypy version used: mypy 1.14.0+dev.2ebc690279c7e20ed8bec006787030c5ba57c40e (compiled: no)
@AlexWaygood AlexWaygood added the bug mypy got something wrong label Nov 12, 2024
@randolf-scholz
Copy link
Contributor

randolf-scholz commented Nov 12, 2024

I think this is because since python 3.12, slice is hashable, hence it satisfies both overloads and mypy performs a join of the return types. When you run with 3.11 the error disappears:

https://mypy-play.net/?mypy=master&python=3.11&gist=339ba7f72c9048770ce15d6dc75207a1

@randolf-scholz
Copy link
Contributor

randolf-scholz commented Nov 12, 2024

@AlexWaygood Uhh, I just discovered that it actually seems to be related to the bug #2410 we just encountered in typeshed as well, because when you use non-literal slices it also disappears!

from typing import assert_type, overload, Hashable

class Foo: ...

class DataFrame:
    @overload
    def __getitem__(self, key: slice, /) -> "DataFrame": ...
    @overload
    def __getitem__(self, key: Hashable, /) -> Foo: ...


df = DataFrame()
assert_type(df[1:], DataFrame)  # ❌
assert_type(df[:2], DataFrame)  # ❌
assert_type(df[slice(1, None)], DataFrame)  # ✅
assert_type(df[slice(None, 2)], DataFrame)  # ✅

https://mypy-play.net/?mypy=master&python=3.12&gist=935d2927539dbe01d6727037ace98469


I was wondering about this because according to https://mypy.readthedocs.io/en/stable/more_types.html#type-checking-calls-to-overloads, no join operation should happen. I guess there is some legacy code responsible for dealing with literal slices in __getitem__ that's going haywire?

@brianschubert

This comment was marked as outdated.

@brianschubert
Copy link
Collaborator

brianschubert commented Nov 12, 2024

Actually, my previous comment wasn't as far from the mark as I thought 😆

The difference between slice expressions and non-literal slices does come down to the hardcoded analysis in the expression checker, specifically the return:

return self.named_type("builtins.slice")

This erases any type information and makes all slice expressions be treated as slice[Any, Any, Any]. Manually passing a slice of type slice[Any, Any, Any] indeed has the same behavior:

df = DataFrame()
x: slice[Any, Any, Any]
assert_type(df[x], DataFrame)  # E: Expression is of type "Any", not "DataFrame"

I'm not fully sure why passing slice[Any, Any, Any] causes the overload return type to resolve to Any, but I think it may have to do with the way infer_overload_return_type resolves ambiguous overloads, specifically now that this check will resolve to True:

args_contain_any = any(map(has_any_type, arg_types))

so this branch can no longer be taken:

mypy/mypy/checkexpr.py

Lines 2882 to 2884 in 2ebc690

if not args_contain_any:
self.chk.store_types(m)
return ret_type, infer_type

I think the fix would be to just properly infer the generic type arguments for slice expressions. Naively that could look something like the following, though we'd probably want to make it match the behavior of slice.__new__ in typeshed.

--- a/mypy/checkexpr.py
+++ b/mypy/checkexpr.py
@@ -5612,11 +5612,15 @@ class ExpressionChecker(ExpressionVisitor[Type]):
         except KeyError:
             supports_index = self.chk.named_type("builtins.int")  # thanks, fixture life
         expected = make_optional_type(supports_index)
+        type_args = []
         for index in [e.begin_index, e.end_index, e.stride]:
             if index:
                 t = self.accept(index)
                 self.chk.check_subtype(t, expected, index, message_registry.INVALID_SLICE_INDEX)
-        return self.named_type("builtins.slice")
+                type_args.append(t)
+            else:
+                type_args.append(NoneType())
+        return self.chk.named_generic_type("builtins.slice", type_args)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

3 participants