Convert selected ABCs to Protocols #1220

ilevkivskyi · 2017-04-26T16:52:00Z

A basic set of updates to typing that converts selected ABCs to protocols, see PEP 544.

This accompanies python/mypy#3132 and python/typing#417
The tests will fail, so that this PR can be merged after the mypy one.
Then, if everything goes OK, we could convert more types in typeshed to protocols.

Attn: @JukkaL @ambv @gvanrossum

JelleZijlstra · 2017-04-26T17:04:20Z

stdlib/2/typing.pyi

@@ -16,6 +16,7 @@ Type = object()
 _promote = object()
 no_type_check = object()
 ClassVar = object()
+Protocol = object()


I know we have a lot of similar definitions already, but I'd prefer to keep the stub closer to reality, maybe with just class Protocol: pass

This form is actually necessary (in some sense) since mypy will prohibit any other operations with Protocol in runtime context, like:

Protocol() # instantiating is a runtime error Protocol[int] # type application is a runtime error def make_one(cls: type) -> Any: ... make_one(Protocol) # passing to a function is not a good idea def fun(x: Protocol) -> int: # Using it in type context is not a good idea either ...

This could be fixed by additional special-casing, but a simpler way is just make it an object().

It's special either way: if I do Protocol = object() elsewhere, mypy probably won't let me do class SupportsFloat(Protocol): pass. Using a class is at least a little closer to what Protocol actually does at runtime, even if Protocol obviously isn't a normal class.

Yes, but if I write class Protocol: ... then mypy will allow most things I mentioned above. This is why object() is used also for Generic and friends. Otherwise prohibiting all possible invalid scenarios will require a lot of effort.

Oh I see, I thought it would just ignore the implementation in the stub. Ignore this then.

JelleZijlstra · 2017-05-20T18:29:46Z

This looks good to merge once the mypy PR is in.

I fixed a merge conflict that happened because I added typing.ContextManager. Not sure though if it should have the @runtime decorator.

ilevkivskyi · 2017-05-21T21:48:21Z

Not sure though if it should have the @runtime decorator.

Yes, this is a runtime protocol (since contextlib.AbstractContextManager supports runtime structural checks via __subclasshook__). Also AsyncConetxtManager should be marked as @runtime when added.

JukkaL · 2017-08-10T17:44:43Z

A few notes:

Please don't merge this until you get an okay here from me, even if the corresponding mypy PR is merged.
I think that the explicit SupportsFoo base classes in the stubs for builtins (for example, SupportsInt in int) should be removed. The rationale is that they are unnecessary as these are now protocols, and they also confuse mypy type inference as implemented in Protocols mypy#3132 by generating inferred types such as List[SupportsInt] instead List[object] that are unexpected.

ilevkivskyi · 2017-08-10T20:21:47Z

OK, I have updated builtins (removed the Supports* bases).

ilevkivskyi · 2017-08-19T12:36:45Z

OK, now all tests pass except for one. We are now in an interesting situation, since pythoneval tests are run with full stubs, this means that I cannot fix this in mypy before this one is merged and vice-versa. This means that either mypy of typeshed would need to merge a failing PR or this will involve a long loop of PRs with temporarily disabling this test in mypy then merging this PR and then re-enabling this one test again. @JelleZijlstra how such situations are typically treated?

I think this can be considered unblocked and ready for review, since python/mypy#3132 is now merged. Also, we could think now what are other classes in stdlib that may be made protocols (maybe PathLike?), and where removing an explicit bases could improve situation (for example, currently mypy does not attempt structural check, if there is an explicit base, and subtype check fails because of type variables, variance, etc.)

JelleZijlstra · 2017-08-19T14:22:38Z

stdlib/2/typing.pyi

@@ -62,46 +63,58 @@ _KT_co = TypeVar('_KT_co', covariant=True)  # Key type covariant containers.
 _VT_co = TypeVar('_VT_co', covariant=True)  # Value type covariant containers.
 _T_contra = TypeVar('_T_contra', contravariant=True)  # Ditto contravariant.

-class SupportsInt(metaclass=ABCMeta):
+def runtime(cls: _T) -> _T: ...


Should the argument be a TypeVar bound to Type[object] or even Type[Protocol], so that type checkers will give an error when this decorator is applied to something that's not a class?

Type[Protocol] will not work, since it does not appear explicitly in MRO, but Type[object] looks like a good idea.

JelleZijlstra · 2017-08-19T14:23:50Z

stdlib/3/typing.pyi

        def __aenter__(self) -> Awaitable[_T_co]: ...
        def __aexit__(self, exc_type: Optional[Type[BaseException]],
                      exc_value: Optional[BaseException],
                      traceback: Optional[TracebackType]) -> Awaitable[Optional[bool]]: ...

-class Mapping(_Collection[_KT], Generic[_KT, _VT_co]):
+@runtime  # type: ignore


Why do you need the # type: ignore here?

Actually, it looks like is is not necessary anymore. The point is that Mapping is not really covariant in value (see signature of get) and mypy is deliberately very strict about such situations with protocols, however it looks like treatment of unions has changed since this was added. I will remove it just in case.

Weird, somehow this works in Python 3 but fails in Python 2, will investigate later, have to run now.

Ah, found it, the return types for values() etc. are list[_VT_co], so on Python 2 it is indeed clearly invariant in value and we need #type: ignore.

JelleZijlstra · 2017-08-19T14:26:15Z

I guess it makes the most sense to have the same person merge the mypy and typeshed changes at approximately the same time. I believe Jukka has push privileges to typeshed, so it might make the most sense for him to do it. I could also take care of doing the actual merges if Jukka or another mypy core team member approves the corresponding mypy PR.

As for uses of protocols, my recent experience has been that protocols would be mostly useful in IO-related argument types, where now we often have to write IO when really we only need an object with a .read() method. I'm on really bad airport wifi now so I can't look up examples easily.

ilevkivskyi · 2017-08-19T19:34:45Z

@JelleZijlstra @JukkaL OK, here is the corresponding mypy PR with an updated test output python/mypy#3846

ilevkivskyi · 2017-11-17T12:55:13Z

@JelleZijlstra @JukkaL
OK, I have removed Sequence and Mapping from this PR as @gvanrossum proposed in python/typing#11 (comment) so this PR can be merged now. Sequence and Mapping can be considered later, since they are not yet protocols at runtime.

The logistic issue here is that this PR and python/mypy#3846 mutually depend on each other. As proposed above the simplest way would be to just merge them simultaneously.

gvanrossum

See also my suggestion at python/mypy#3846 (comment) on how to avoid the mutual dependency between the two PRs.

Finally, can you rebase? There are no merge conflicts but it's really old, which surprised me when testing this manually. (OTOH the manual test on internal codebases found no new issues, so green light from that POV.)

gvanrossum · 2017-11-17T23:26:45Z

stdlib/2/typing.pyi

-class SupportsInt(metaclass=ABCMeta):
+def runtime(cls: _TC) -> _TC: ...
+
+@runtime


Since this is a stub, why does it need @runtime?

Otherwise mypy will complain about e.g. issubclass(it, collections.abc.Iterable) something like isinstance() can be used only with @runtime protocols.

gvanrossum · 2017-11-17T23:27:08Z

stdlib/2/typing.pyi


-class SupportsInt(metaclass=ABCMeta):
+def runtime(cls: _TC) -> _TC: ...


IIUC @runtime is not defined (yet) in typing.py but (only) in typing_extensions.py?

Yes, and the same for Protocol (there is only old internal _Protocol in typing). We however need them in typeshed to avoid circular dependencies with typing_extensions. We discussed this with @JukkaL few moths ago, the plan (which I like) is this:

Iterate on PR Make python 2 subprocess consistent with python 3 #3132 until it's approved.

Publish Protocol in typing_extensions. Also include it in the stub for typing_extensions.

Merge PR Make python 2 subprocess consistent with python 3 #3132 but don't merge typeshed changes yet. This lets mypy users (at least those who use GitHub master) experiment with user-defined protocols.

Get an okay from PyCharm and Pytype that they are fine with merging the typeshed PR. They might need to implement some support for the Protocol special form first, but if they don't use the typing typeshed stub this may be a non-issue.

If major issues are found in the mypy implementation of protocols, fix them.

Release a new mypy version with protocols as an experimental feature!

Merge the typeshed PR and use it in mypy.

Seek acceptance of the protocols PEP once we have some practical experience and protocols seem to work fine.

Officially support using protocols in typeshed once Pytype and PyCharm are okay with it.

Release Protocol in Python stdlib typing module.

This PR is step number 7

ilevkivskyi · 2017-11-19T08:28:14Z

@gvanrossum OK, I rebased (but also squashed, since there were previous merge conflicts I don't want to fix them all again).

ilevkivskyi · 2017-11-19T09:16:38Z

OK, all tests pass now, this is ready to be merged.

…ols) (#3846) This PR contains the tests output after python/typeshed#1220 is merged and synced (note that pythoneval tests are run with full typeshed stubs).

JelleZijlstra reviewed Apr 26, 2017

View reviewed changes

JelleZijlstra added the blocked label May 20, 2017

JukkaL mentioned this pull request Aug 10, 2017

Protocols python/mypy#3132

Merged

JelleZijlstra reviewed Aug 19, 2017

View reviewed changes

ilevkivskyi mentioned this pull request Aug 19, 2017

Update pythoneval test according to PR typeshed/#1220 (ABCs -> Protocols) python/mypy#3846

Merged

gvanrossum changed the title ~~Protocols~~ Convert selected ABCs to Protocols Sep 19, 2017

ilevkivskyi mentioned this pull request Sep 23, 2017

bytes(obj) rejected even when obj implements __bytes__ python/mypy#3989

Closed

ilevkivskyi mentioned this pull request Nov 1, 2017

Protocols (a.k.a. structural subtyping) python/typing#11

Closed

gvanrossum reviewed Nov 17, 2017

View reviewed changes

ilevkivskyi force-pushed the protocols branch from b3bcce4 to 4cfddfd Compare November 19, 2017 08:24

Recognize standard ABCs in typing as protocols

f6d7dea

ilevkivskyi force-pushed the protocols branch from 4cfddfd to f6d7dea Compare November 19, 2017 08:32

Restart tests

b2fc6db

gvanrossum merged commit ec2cb8e into python:master Nov 19, 2017

ilevkivskyi deleted the protocols branch November 19, 2017 17:51

ilevkivskyi mentioned this pull request Nov 19, 2017

Sync typeshed python/mypy#4267

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert selected ABCs to Protocols #1220

Convert selected ABCs to Protocols #1220

ilevkivskyi commented Apr 26, 2017

JelleZijlstra Apr 26, 2017

ilevkivskyi Apr 26, 2017 •

edited

Loading

JelleZijlstra Apr 26, 2017

ilevkivskyi Apr 26, 2017 •

edited

Loading

JelleZijlstra Apr 26, 2017

JelleZijlstra commented May 20, 2017

ilevkivskyi commented May 21, 2017

JukkaL commented Aug 10, 2017

ilevkivskyi commented Aug 10, 2017

ilevkivskyi commented Aug 19, 2017

JelleZijlstra Aug 19, 2017

ilevkivskyi Aug 19, 2017

JelleZijlstra Aug 19, 2017

ilevkivskyi Aug 19, 2017

ilevkivskyi Aug 19, 2017

ilevkivskyi Aug 19, 2017

JelleZijlstra commented Aug 19, 2017

ilevkivskyi commented Aug 19, 2017

ilevkivskyi commented Nov 17, 2017

gvanrossum left a comment •

edited

Loading

gvanrossum Nov 17, 2017

ilevkivskyi Nov 19, 2017

gvanrossum Nov 17, 2017

ilevkivskyi Nov 19, 2017

ilevkivskyi commented Nov 19, 2017

ilevkivskyi commented Nov 19, 2017


		class SupportsInt(metaclass=ABCMeta):
		def runtime(cls: _TC) -> _TC: ...

Convert selected ABCs to Protocols #1220

Convert selected ABCs to Protocols #1220

Conversation

ilevkivskyi commented Apr 26, 2017

Choose a reason for hiding this comment

ilevkivskyi Apr 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi Apr 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JelleZijlstra commented May 20, 2017

ilevkivskyi commented May 21, 2017

JukkaL commented Aug 10, 2017

ilevkivskyi commented Aug 10, 2017

ilevkivskyi commented Aug 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JelleZijlstra commented Aug 19, 2017

ilevkivskyi commented Aug 19, 2017

ilevkivskyi commented Nov 17, 2017

gvanrossum left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi commented Nov 19, 2017

ilevkivskyi commented Nov 19, 2017

ilevkivskyi Apr 26, 2017 •

edited

Loading

ilevkivskyi Apr 26, 2017 •

edited

Loading

gvanrossum left a comment •

edited

Loading