new protocol, overloads to list.sort, sorted; prep. to fix #4051, #4155
Conversation
|
Ok, I am officially stuck. I there is a Python 3.9 stub test that keeps failing. I don't know how to fix that. Apparently @hauntsaninja found something wrong with the Python 3.9 on the CI, but I don't know how he fixed his build right after mine. Thanks for any help. |
|
Sorry about this! |
ramalho
changed the title
stdlib/2/__builtin__.pyi
Outdated
| @overload | ||
| def sort(self: List[_LT], *, key: None = ..., reverse: bool = ...) -> None: ... | ||
| @overload | ||
| def sort(self, *, key: Callable[[_T], _LT], reverse: bool = ...) -> None: ... |
There was a problem hiding this comment.
Since you are only using one instance of _LT in each overload, the type var is redundant. Just using _SupportsLessThan should work.
There was a problem hiding this comment.
Thanks for pointing this out @srittau , I will change that in my next commit.
| def sorted(__iterable: Iterable[_T], *, | ||
| key: Optional[Callable[[_T], Any]] = ..., | ||
| key: Callable[[_T], _LT], |
There was a problem hiding this comment.
I will implement this change as well. Thanks, @srittau !
There was a problem hiding this comment.
I agree with @srittau here. A typevar used in a single position doesn't make semantic sense and type checkers could reasonably throw an error for code like this.
There was a problem hiding this comment.
But here there is no invariance problem, so the _LT can just be _SupportsLessThan.
|
Hmm, mypy's throwing a lot of |
|
Ok, my bad. Seems we should go back to the typevar for now. We can still fix this later when it gets fixed in mypy. |
|
Oh, that's probably because List is invariant, so |
| @@ -943,7 +947,10 @@ class list(MutableSequence[_T], Generic[_T]): | |||
| def remove(self, __value: _T) -> None: ... | |||
| def reverse(self) -> None: ... | |||
| if sys.version_info >= (3,): | |||
| def sort(self, *, key: Optional[Callable[[_T], Any]] = ..., reverse: bool = ...) -> None: ... | |||
| @overload | |||
| def sort(self: List[_SupportsLessThan], *, key: None = ..., reverse: bool = ...) -> None: ... | |||
There was a problem hiding this comment.
This must actually be List[_LT] to avoid the invariance issue mentioned in my comment.
| def sorted(__iterable: Iterable[_T], *, | ||
| key: Optional[Callable[[_T], Any]] = ..., | ||
| key: Callable[[_T], _LT], |
There was a problem hiding this comment.
But here there is no invariance problem, so the _LT can just be _SupportsLessThan.
|
Hello, @JelleZijlstra, @srittau, and @hauntsaninja. Thanks for your help with this PR. I made a mistake here and I had to start a new PR #4192, which is passing all checks. I am closing this PR. Please let's continue at #4192. Thanks! |
As agreed with @JelleZijlstra, this PR introduces the
_SupportsLessThanprotocol and a type variable bounded to it to check arguments tolist.sortandsortedon Python 3 (on Python 2, these checks do not reflect reality: everything supports__lt__, evenNoneandobject).When this PR is merged, I will send another one using
_SupportsLessThanto check arguments tominandmax, fixing #4051 with lots of overloads.