-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Update docs to reflect the fact that Iterable etc. are protocols #4344
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great improvement to the docs!
|
||
.. code-block:: python | ||
|
||
def __await__(self) -> Generator[Any, None, T] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can write this as:
async def __await__(self) -> T
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer Jukka's version, actually. (Though maybe we can explain the two are equivalent? This is an area that few people are familiar with.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just know that some people (for example @1st1) don't like mentioning generators in async docs.
these default implementations. Explicitly including a protocol as a | ||
base class is also a way of documenting that your class implements a | ||
particular protocol, and it forces mypy to verify that your class | ||
implementation is actually compatible with the protocol. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a caveat that this still works (although it shouldn't according to PEP 544):
class P(Protocol):
def meth(self) -> int: ... # No error here...
class C(P):
pass
C() # ...and no error here!
mypy currently treats empty body (ellipsis) as a default implementation. There is an issue to allow this only in stubs (and in tests), but it is not implemented yet. (The current workaround is to use @abstractmethod
, IIRC the similar problem with literal ellipsis in x: int = ...
is already fixed.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to call this out in the docs. I do think it's important to fix. (Honestly I don't understand the difference between the two issues you link to.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Honestly I don't understand the difference between the two issues you link to.)
They are duplicates, but both have some discussion, so I don't know which one to close.
typing, which is well known to Python programmers. Mypy provides an | ||
opt-in support for structural subtyping via protocol classes described | ||
typing, which is well known to Python programmers. Mypy provides | ||
support for structural subtyping via protocol classes described |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicking question: the part about duck typing is technically not true (e.g. cross relations between variables are not captured even when statically analysable). What does it give to the reader that is not conveyed by the rest of the discussion? If all we want is help readers that search for the term "duck typing", perhaps we can phrase is differently - maybe "Structural subtyping is a closer approximation of duck typing than inheritance" or some other weak claim.
(Sorry, I know this is not important; the rest of the PR LGTM).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the lack of 100% full equivalency is not something that needs to be called out here. In general mypy disapproves of many code patterns that "work" and we don't call those out specifically as deficiencies elsewhere.
typing, which is well known to Python programmers. Mypy provides an | ||
opt-in support for structural subtyping via protocol classes described | ||
typing, which is well known to Python programmers. Mypy provides | ||
support for structural subtyping via protocol classes described |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the lack of 100% full equivalency is not something that needs to be called out here. In general mypy disapproves of many code patterns that "work" and we don't call those out specifically as deficiencies elsewhere.
|
||
.. code-block:: python | ||
|
||
def __await__(self) -> Generator[Any, None, T] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer Jukka's version, actually. (Though maybe we can explain the two are equivalent? This is an area that few people are familiar with.)
these default implementations. Explicitly including a protocol as a | ||
base class is also a way of documenting that your class implements a | ||
particular protocol, and it forces mypy to verify that your class | ||
implementation is actually compatible with the protocol. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to call this out in the docs. I do think it's important to fix. (Honestly I don't understand the difference between the two issues you link to.)
Also update the discussion of ABCs.
Also update the discussion of ABCs, and don't describe protocols as
experimental.
Fixes #4339.