Collection is Sized - #8977
Collection is Sized#8977JelleZijlstra merged 7 commits into
Conversation
Sorry, something went wrong.
hauntsaninja
left a comment
There was a problem hiding this comment.
Hm, the decision not to implement Sized in Collection seems pretty intentional.
Looks like this goes back to #2658 , and was maybe done for Python 2 reasons?
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
srittau
left a comment
There was a problem hiding this comment.
Considering that primer does not complain and the change back in 2018 alludes to Python 2/3 problems or problems that may be fixed in mypy 0.650, this looks good to me. But maybe @gvanrossum has some recollection about this?
Sorry, something went wrong.
|
This change seems fine, since the original motivation was Python 2/3 compatibility concerns, and the mypy primer output is clean. |
Sorry, something went wrong.
|
I think that it is a good idea to copy original samples to our test cases. |
Sorry, something went wrong.
|
pyright has a different opinion: /home/runner/work/typeshed/typeshed/test_cases/stdlib/typing/check_aliases.py:4:13 - error: "assert_type" mismatch: expected "list[Collection[str]]" but received "list[str | set[str]]" Ok, a test case won't quite work. But, here's my manual test case for this: (.venv) ~/Desktop/typeshed collection-is-sized ✔
» mypy -c "reveal_type(['a', {'b'}])" --custom-typeshed-dir .
<string>:1: note: Revealed type is "builtins.list[typing.Collection[builtins.str]]"
Success: no issues found in 1 source file
(.venv) ~/Desktop/typeshed collection-is-sized ✔
» mypy -c "reveal_type(['a', ['b']])" --custom-typeshed-dir .
<string>:1: note: Revealed type is "builtins.list[typing.Sequence[builtins.str]]"
Success: no issues found in 1 source file
|
Sorry, something went wrong.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Sorry, something went wrong.
|
Looks like this change breaks mypy: python/mypy#13977 Reproduce with e.g. mypy -c 'reveal_type(5)' --no-silence-site-packages --custom-typeshed-dir ~/dev/typeshed The two quickest fixes are either: diff --git a/stdlib/typing.pyi b/stdlib/typing.pyi
index d03a3eab..c9cfe304 100644
--- a/stdlib/typing.pyi
+++ b/stdlib/typing.pyi
@@ -325,7 +325,7 @@ class SupportsRound(Protocol[_T_co]):
def __round__(self, __ndigits: int) -> _T_co: ...
@runtime_checkable
-class Sized(Protocol, metaclass=ABCMeta):
+class Sized(Protocol):
@abstractmethod
def __len__(self) -> int: ...
or: diff --git a/stdlib/typing.pyi b/stdlib/typing.pyi
index d03a3eab..bfb7cfa7 100644
--- a/stdlib/typing.pyi
+++ b/stdlib/typing.pyi
@@ -452,7 +452,9 @@ class Container(Protocol[_T_co]):
def __contains__(self, __x: object) -> bool: ...
@runtime_checkable
-class Collection(Sized, Iterable[_T_co], Container[_T_co], Protocol[_T_co]): ...
+class Collection(Iterable[_T_co], Container[_T_co], Protocol[_T_co]):
+ @abstractmethod
+ def __len__(self) -> int: ...
class Sequence(Collection[_T_co], Reversible[_T_co], Generic[_T_co]):
@overload
Which do people prefer? |
Sorry, something went wrong.
|
I'd prefer removing the metaclass, I don't think we need it there. |
Sorry, something went wrong.
|
I think this change caused a false positive error that I'm seeing. More info in #9296. |
Sorry, something went wrong.
This reverts commit 5bbba5d.
See https://github.com/python/cpython/blob/ad1dc3ebb6aadaeeeacde13d4ed2d62bf302bf62/Lib/_collections_abc.py#L431