Fix type argument inference for overloaded functions with explicit self types (Fixes #14943). by tyralla · Pull Request #14975 · python/mypy · GitHub

Fix type argument inference for overloaded functions with explicit self types (Fixes #14943).

When finding no compatible callable right away, try to let method ConstraintBuilderVisitor.find_matching_overload_item return the first overloaded function with the suitable self type.

Checked by test case TestOverloadedMethodWithExplictSelfTypes.

…lf types (Fixes python#14943). When finding no compatible callable right away, try to let method `ConstraintBuilderVisitor.find_matching_overload_item` return the first overloaded function with the suitable self type. Checked by test case `TestOverloadedMethodWithExplictSelfTypes`.

The definition attribute is only supposed to be used for error messages, and it's not guaranteed to be set. It cannot be reliably used for type checking. It's not serialized, in particular, but there may well be other contexts where it's lost that I'm not aware of.

Also, the bound_args attribute is unfortunately not documented (or the documentation seems incorrect). Again, I'm not sure if we can expect it to be always set.

Here's an alternative approach that might work. When we bind the self type to an overloaded method (in mypy.typeops.bind_self), what about filtering the overload items at that point to those matching the self type (only when explicit self types are used)? This filtering would happen in an earlier stage, and I believe we have access to all the necessary information without using undocumented or inconsistently set attributes. The result of the filtering could be a non-overloaded callable type or an overloaded type with fewer items.

Thank you for your feedback. Using two fragile attributes in five new lines - what a negative hit rate...

Your suggestion targets code sections I did not encounter yet. My first naive implementation of your suggestion solved the original issue but broke many other things. So far, I could not find any information to distinguish implicit and explicit self types within bind_self. Is there some obvious way I am missing?

I would take a second, more thorough try within the next few days. Please do not hesitate to fix this yourself if you already have a good solution in mind and if you want that issue fixed before the 1.2 release.

I tried to filter within bind_self as suggested. All tests pass (on my computer). Let's see what Mypy primer says.

I am still not overly happy with my approach for the following reasons:

I added the ignore_type_vars option to is_subtype, which seems like a huge change for such a specific problem to me.
I found no way to distinguish explicit self types from implicit self types.
The result is still always an overloaded type (changing this within bind_self would be simple but requires changing its signature and eventually many code sections that use bind_self).

3. The result is still always an overloaded type (changing this within bind_self would be simple but requires changing its signature and eventually many code sections that use bind_self).

I played around with returning a single non-overloaded callable for a few minutes. Maybe it's not a good idea because it changes the signatures in error reports and notes. On the other hand, I did not encounter a case where an overloaded type with a single item caused a problem (but I did not actively search for such a problem).

(drive by comment, will look further when I get time)
Hmm, I'd added logic like this in #14017 and later moved it to bind_and_map_method in #14882. Maybe there's some unification to be had

… from `testSelfTypeOverrideCompatibilityTypeVar`

Hmm, I'd added logic like this in #14017 and later moved it to bind_and_map_method in #14882. Maybe there's some unification to be had

Your tip seems helpful. I could remove the filtering logic you implemented in checker.TypeChecker.bind_and_map_method without breaking any tests. Also, testSelfTypeOverrideCompatibilityTypeVar, previously marked by "xfail", now passes. However, this benefit still comes at the cost of the ignore_type_vars approach (which might eventually shadow some actual bugs?).

Yeah, I'm not sure about the ignore_type_vars

See this other related PR I opened here: #15045 In this case I was able to replace type vars by their upper bound

…n from python#15045 for comparison)

I added the test case you introduced in #15045 for testing (I can remove it later). It passes with the current ignore_type_vars approach.

For further testing, I also disabled the ignore_type_vars option (not committed). This results only in problems for the test cases testSelfTypeRestrictedMethodOverloadInit and testSelfTypeRestrictedMethodOverloadInitFallBacks, which check the following modifications of the self type:

class C(Generic[T]):
    @overload
    def __init__(self: C[T], item: T, use_tuple: Literal[False]) -> None: ...
    @overload
    def __init__(self: C[Tuple[T, ...]], item: T, use_tuple: Literal[True]) -> None: ...

Maybe assuming general TypeVar compatibility in the given context is not such a bad thing because Tuple[T, ...]] could be replaced by arbitrarily complex expressions. And actual bugs (e.g. because T is bound to something else) should be found elsewhere. Or can you think of an example where the ignore_type_vars approach would not remove an overloaded item with a well-defined explicit self type that does not fit the current "original type"?

I think that test case is buggy, there was some discussion at #14729 (comment) and python/typing#1340 (comment)

Interesting. If we would disallow such self type definitions, the ignore_type_vars option would hopefully become obsolete. I commit this change to see the results on the primer.

Diff from mypy_primer, showing the effect of this PR on open source code:

pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/series.py:3330: error: Signature of "sort_values" incompatible with supertype "NDFrame"  [override]
+ pandas/core/series.py:3330: note:      Superclass:
+ pandas/core/series.py:3330: note:          @overload
+ pandas/core/series.py:3330: note:          def sort_values(self, *, axis: Union[int, Literal['index', 'columns', 'rows']] = ..., ascending: Union[bool, Sequence[bool]] = ..., inplace: Literal[False] = ..., kind: str = ..., na_position: str = ..., ignore_index: bool = ..., key: Optional[Callable[[Series], Union[Series, Union[Union[ExtensionArray, ndarray[Any, Any]], Index, Series]]]] = ...) -> Series
+ pandas/core/series.py:3330: note:          @overload
+ pandas/core/series.py:3330: note:          def sort_values(self, *, axis: Union[int, Literal['index', 'columns', 'rows']] = ..., ascending: Union[bool, Sequence[bool]] = ..., inplace: Literal[True], kind: str = ..., na_position: str = ..., ignore_index: bool = ..., key: Optional[Callable[[Series], Union[Series, Union[Union[ExtensionArray, ndarray[Any, Any]], Index, Series]]]] = ...) -> None
+ pandas/core/series.py:3330: note:          @overload
+ pandas/core/series.py:3330: note:          def sort_values(self, *, axis: Union[int, Literal['index', 'columns', 'rows']] = ..., ascending: Union[bool, Sequence[bool]] = ..., inplace: bool = ..., kind: str = ..., na_position: str = ..., ignore_index: bool = ..., key: Optional[Callable[[Series], Union[Series, Union[Union[ExtensionArray, ndarray[Any, Any]], Index, Series]]]] = ...) -> Optional[Series]
+ pandas/core/series.py:3330: note:      Subclass:
+ pandas/core/series.py:3330: note:          @overload
+ pandas/core/series.py:3330: note:          def sort_values(self, *, axis: Union[int, Literal['index', 'columns', 'rows']] = ..., ascending: Union[bool, int, Sequence[bool], Sequence[int]] = ..., inplace: Literal[False] = ..., kind: str = ..., na_position: str = ..., ignore_index: bool = ..., key: Optional[Callable[[Series], Union[Series, Union[Union[ExtensionArray, ndarray[Any, Any]], Index, Series]]]] = ...) -> Series
+ pandas/core/series.py:3330: note:          @overload
+ pandas/core/series.py:3330: note:          def sort_values(self, *, axis: Union[int, Literal['index', 'columns', 'rows']] = ..., ascending: Union[bool, int, Sequence[bool], Sequence[int]] = ..., inplace: Literal[True], kind: str = ..., na_position: str = ..., ignore_index: bool = ..., key: Optional[Callable[[Series], Union[Series, Union[Union[ExtensionArray, ndarray[Any, Any]], Index, Series]]]] = ...) -> None

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
- pandas-stubs/core/indexes/range.pyi:87: error: Unused "type: ignore" comment

cwltool (https://github.com/common-workflow-language/cwltool)
- cwltool/main.py:964: error: Unused "type: ignore[arg-type]" comment

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
- bson/son.py: note: At top level:
- bson/son.py:118: error: Unused "type: ignore" comment

The ignore_type_vars=False experiment showed that type_object_type_from_function also required some adjustment, as it relies on pre-determined self types. Many problems reported by the primer (at least those related to defaultdict) were due to filtering the overloaded items but not their pre-determined self types. I fixed this, but so far likely not in the best possible style.

However, some other findings gave me the impression that the ignore_type_vars approach might really be okay, so I reactivated it. Everything else I tried had its edges but no advantages I am aware of.

This requires an explicit `cast` of `shutil.copyfileobj` to work-around a typeshed/Mypy issue. Bumps [mypy](https://github.com/python/mypy) from 1.1.1 to 1.4.1. - [Commits](python/mypy@v1.1.1...v1.4.1) --- updated-dependencies: - dependency-name: mypy dependency-type: direct:development update-type: version-update:semver-minor ... See: python/mypy#15031 See: https://github.com/python/mypy/pull/14902/files#diff-363460977156fcfda748f21565484fe1d5862edf2823e9784cbf34d1e52ff2f2 See: python/mypy#14975 Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Nicolas Trangez <ikke@nicolast.be>

Fix type argument inference for overloaded functions with explicit self types. Filter out the overload items based on the declared and actual types of self. The implementation is best effort and does the filtering only in simple cases, to reduce the risk of regressions (primarily performance, but I worry also about infinite recursion). I added a fast path for the typical case, since without it the filtering was quite expensive. Note that the overload item filtering already worked in many contexts. This only improves it in specific contexts -- at least when inferring generic protocol compatibility. This is a more localized (and thus lower-risk) fix compared to #14975 (thanks @tyralla!). #14975 might still be a good idea, but I'm not comfortable merging it now, and I want a quick fix to unblock the mypy 1.12 release. Fixes #15031. Fixes #17863. Co-authored by @tyralla.

The latest on this is #17873 (and before that #15045 was also related)

I guess it becomes more and more unlikely this will ever be merged. So conflict solving and investigating if the ignore_typ_vars is still really beneficial somehow, given the fixes in the mentioned PRs, seems like a waste of time. Shall we just close this PR?

AlexWaygood requested a review from JukkaL March 29, 2023 15:46

AlexWaygood mentioned this pull request Apr 4, 2023

Release 1.2 planning #14916

Closed

2 tasks done

JukkaL reviewed Apr 5, 2023

View reviewed changes

tyralla added 4 commits April 11, 2023 00:06

revert first attempt

f354153

try filtering in bind_self instead

589e0c6

black

9c107b3

flake

b143796

This comment has been minimized.

Sign in to view

AlexWaygood mentioned this pull request Apr 11, 2023

shutil.copyfileobj stopped working in 1.2.0 #15031

Closed

hauntsaninja reviewed Apr 11, 2023

View reviewed changes

Simplify checker.TypeChecker.bind_and_map_method and remove "xfail"…

6b1573f

… from `testSelfTypeOverrideCompatibilityTypeVar`

This comment has been minimized.

Sign in to view

hauntsaninja added the upnext label Apr 14, 2023

Add testSelfTypeOverrideCompatibilitySelfTypeVar (not my work: take…

f1414f2

…n from python#15045 for comparison)

This comment has been minimized.

Sign in to view

ignore_type_vars=False for testing

6e5476a

This comment has been minimized.

Sign in to view

tyralla added 2 commits April 16, 2023 01:15

Also filter externally defined self types (preliminary approach)

3097ac2

reverse ignore_type_vars=False

33a1f96

hauntsaninja mentioned this pull request Apr 18, 2023

Fix bounded self types in override incompatibility checking #15045

Merged

AlexWaygood mentioned this pull request May 11, 2023

Cannot infer type argument 1 of "shutil.copyfileobj" with mypy 0.780 python/typeshed#4201

Closed

JukkaL mentioned this pull request Oct 3, 2024

Filter overload items based on self type during type inference #17873

Merged

JukkaL added a commit that referenced this pull request Oct 3, 2024

Add test case from #14975 by @tyralla

86c1124

hauntsaninja removed the upnext label Nov 3, 2024

tyralla closed this May 22, 2025

+                  # Try to return the first item with the correct self type (fixes issue 14943).
+                  for item in items:
+                      if isinstance(item.definition, FuncDef) and isinstance(item.definition.type, CallableType):
+                          if item.bound_args and item.definition.type.arg_types:

Fix type argument inference for overloaded functions with explicit self types (Fixes #14943). by tyralla · Pull Request #14975 · python/mypy · GitHub

Repository navigation

Fix type argument inference for overloaded functions with explicit self types (Fixes #14943). - #14975

Conversation

tyralla commented Mar 29, 2023

Uh oh!

JukkaL Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

tyralla Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

tyralla Apr 10, 2023

Choose a reason for hiding this comment

Uh oh!

tyralla Apr 15, 2023

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

hauntsaninja left a comment

Choose a reason for hiding this comment

Uh oh!

tyralla commented Apr 14, 2023

Uh oh!

This comment has been minimized.

hauntsaninja commented Apr 14, 2023

Uh oh!

This comment has been minimized.

tyralla commented Apr 15, 2023

Uh oh!

hauntsaninja commented Apr 15, 2023

Uh oh!

tyralla commented Apr 15, 2023

Uh oh!

This comment has been minimized.

github-actions Bot commented Apr 15, 2023

Uh oh!

tyralla commented Apr 15, 2023

Uh oh!

hauntsaninja commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tyralla commented Nov 3, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hauntsaninja commented Nov 3, 2024 •
edited
Loading