KEMBAR78
Show all overloads in error message by Akuli · Pull Request #9177 · python/mypy · GitHub
Skip to content

Conversation

Akuli
Copy link
Contributor

@Akuli Akuli commented Jul 20, 2020

closes #9175

@JukkaL
Copy link
Collaborator

JukkaL commented Jul 31, 2020

Thanks for the PR!

I think that we can just always, unconditionally show all overloads. It may be bit verbose, but probably better since otherwise we'd be hiding important information. -v is mostly used for debugging issues with mypy or configuration, and I'd rather not recommend users to use it for basic use cases.

Could you simplify the PR to just make the change unconditional?

@Akuli
Copy link
Contributor Author

Akuli commented Jul 31, 2020

What should I do in pretty_overload_matches? Show the matching overloads but not the non-matching overloads?

@Akuli
Copy link
Contributor Author

Akuli commented Jul 31, 2020

I was quite confused about what that method does, so let me ask differently: should it show all overloads, or should it show only the overloads that have the correct number of arguments but the arguments have wrong types?

@Akuli Akuli changed the title Show all overloads in error message if -v is given Show all overloads in error message Jul 31, 2020
@Akuli
Copy link
Contributor Author

Akuli commented Jul 31, 2020

I made it show all the overloads

@Akuli
Copy link
Contributor Author

Akuli commented Aug 16, 2020

Is there something else I need to do to get this merged?

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 21, 2021

Sorry, I lost track of this PR. If you can fix the merge conflict, I can do the final review round now.

@Akuli
Copy link
Contributor Author

Akuli commented Sep 21, 2021

I no longer need this PR, because I decided not to abuse overloads for what I was going to do with tkinter, but I'll try to get this fixed.

@Akuli
Copy link
Contributor Author

Akuli commented Sep 21, 2021

Fixing the unit tests will take some time, many are failing :)

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

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

pyppeteer (https://github.com/pyppeteer/pyppeteer.git)
- pyppeteer/network_manager.py:157: note: Possible overload variant:
+ pyppeteer/network_manager.py:157: note: Possible overload variants:
- pyppeteer/network_manager.py:157: note:     <1 more non-matching overload not shown>
+ pyppeteer/network_manager.py:157: note:     def [_T] get(self, key: <nothing>, default: _T) -> _T
- pyppeteer/network_manager.py:158: note: Possible overload variant:
+ pyppeteer/network_manager.py:158: note: Possible overload variants:
- pyppeteer/network_manager.py:158: note:     <1 more non-matching overload not shown>
+ pyppeteer/network_manager.py:158: note:     def [_T] get(self, key: <nothing>, default: _T) -> _T

edgedb (https://github.com/edgedb/edgedb.git)
- edb/server/connpool/pool.py:524:16: note: Possible overload variant:
+ edb/server/connpool/pool.py:524:16: note: Possible overload variants:
- edb/server/connpool/pool.py:524:16: note:     <1 more non-matching overload not shown>
+ edb/server/connpool/pool.py:524:16: note:     def [_T] get(self, key: <nothing>, default: _T) -> _T

pytest (https://github.com/pytest-dev/pytest.git)
+ src/_pytest/main.py:481: note:     def __init__(self) -> _bestrelpath_cache
+ src/_pytest/main.py:481: note:     def __init__(self, **kwargs: str) -> _bestrelpath_cache
- src/_pytest/main.py:481: note:     <2 more non-matching overloads not shown>
- testing/test_config.py:422: error: Too many arguments for "DummyEntryPoint"  [call-arg]

@Akuli
Copy link
Contributor Author

Akuli commented Sep 21, 2021

@JukkaL Ready for review

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Looks good! This makes things more consistent and also simplifies the code.

@JukkaL JukkaL merged commit 579180a into python:master Sep 22, 2021
@Akuli Akuli deleted the overload-verbosity branch September 25, 2021 16:42
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.

show more than two overloads in error message

2 participants