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

Several errors in overloaded functions in the stdlib #9608

Closed
AlexWaygood opened this issue Jan 29, 2023 · 3 comments
Closed

Several errors in overloaded functions in the stdlib #9608

AlexWaygood opened this issue Jan 29, 2023 · 3 comments

Comments

@AlexWaygood
Copy link
Member

AlexWaygood commented Jan 29, 2023

Attempting to add defaults to overloaded functions in #9606 highlighted many instances where we have incorrect overloads in the stdlib. An example is this overload in _bisect.pyi:

@overload
def bisect_left(
a: Sequence[_T],
x: SupportsRichComparisonT,
lo: int = 0,
hi: int | None = None,
*,
key: Callable[[_T], SupportsRichComparisonT] = ...,
) -> int: ...

This overload incorrectly marks the key parameter as having a default value. But this has several issues:

  • If no value is provided for this parameter by the user, we actually want the type checker to pick the first overload (not this overload, which is the second overload). Specifying a default value for this parameter in this overload is confusing.
  • If the type checker did select this overload, the _T TypeVar would go unsolved if no value was specified for the key parameter.
  • If we try to add the default value for this parameter (None), mypy will (correctly) complain that we're using an implicit optional type for the type annotation.

There are several issues like this in our stdlib stubs that should be fixed (see b39305e for a summary of the problematic functions — this is the commit in #9606 in which I reverted all the defaults stubdefaulter had added that caused mypy to emit errors).

@AlexWaygood
Copy link
Member Author

There are several functions in asyncio.events and asyncio.base_events where stubdefaulter added default values that I had to revert (see b39305e). I can see that the overloads are incorrect, but I'm not sure what the correct fix is. I'd appreciate it if somebody who knows their asyncio better than I do could take a look.

@srittau
Copy link
Collaborator

srittau commented Jan 30, 2023

I'm not sure anyone understands asyncio ...

@AlexWaygood
Copy link
Member Author

Other than the asyncio ones, all the fixable ones that I know about have now been fixed (though we'll probably discover a few more once JelleZijlstra/stubdefaulter#35 is implemented...).

I'll open a followup issue for the asyncio ones. Many thanks, everybody, for the speedy reviews on my PRs!

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

No branches or pull requests

2 participants