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

Fixes issue #17840 (shows signature of __call__ for incompatible function argument) #17872

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

aybdee
Copy link

@aybdee aybdee commented Oct 3, 2024

Fixes issue #17840 (shows signature of call for incompatible function argument)
made modifications to messages.py

wrote one unit test in check-callable:testCallableSubtypingTrivialSuffix

This comment has been minimized.

@aybdee
Copy link
Author

aybdee commented Oct 3, 2024

also added added notes to some tests in check-inference.test and check-protocols.test

This comment has been minimized.

Copy link
Collaborator

@brianschubert brianschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this seems to duplicate some logic that's already in incompatible_argument_note, which is called immediately after incompatible_argument. I think a simpler fix would be possible by tweaking incompatible_argument_note instead.

This also seems to be taking the signature from the wrong type. Take for example this test:

[case testCallableImplementsProtocolGenericNotGeneric]
from typing import Protocol, TypeVar, Tuple

T = TypeVar('T')

class Caller(Protocol):
    def __call__(self, x: int) -> int: ...

def call(x: T) -> T: ...

def bad(x: T) -> Tuple[T, T]: ...

def func(caller: Caller) -> None:
    pass

func(call)
func(bad)  # E: Argument 1 to "func" has incompatible type "Callable[[T], Tuple[T, T]]"; expected "Caller" \
           # N: "Caller.__call__" has type "Callable[[Arg(T, 'x')], Tuple[T, T]]"

Here it's displaying the signature of bad instead of Caller.__call__. I would have expected N: "Caller.__call__" has type "Callable[[Arg(int, 'x')], int]

@aybdee
Copy link
Author

aybdee commented Oct 8, 2024

Thank you for the feedback, i've implemented the changes

Copy link
Contributor

github-actions bot commented Oct 8, 2024

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

sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/application.py:1093:38: note: "RoleFunction.__call__" has type "Callable[[str, str, str, int, Inliner, DefaultArg(dict[str, Any] | None, 'options'), DefaultArg(Sequence[str], 'content')], tuple[list[Node], list[system_message]]]"

werkzeug (https://github.com/pallets/werkzeug)
+ tests/test_formparser.py:436: note: "TStreamFactory.__call__" has type "Callable[[Arg(int | None, 'total_content_length'), Arg(str | None, 'content_type'), Arg(str | None, 'filename'), DefaultArg(int | None, 'content_length')], IO[bytes]]"

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

Successfully merging this pull request may close these issues.

2 participants