Protocols by ilevkivskyi · Pull Request #3132 · python/mypy · GitHub

This is an implementation of protocols proposed in PEP 544. This PR adds support for:

generic protocols (including inference for protocols)
recursive protocols
special support for Type[] and isinstance()
structural subtyping for Callable and Tuple
other things

For example, now one can do this:

class Linked(Protocol[T]):
    val: T
    def next(self) -> 'Linked[T]': ...

class L:
    val: int
    def next(self) -> 'L': ...

def last(seq: Linked[T]) -> T:
    ...

reveal_type(last(L()))  # Revealed type is 'builtins.int*'

In general, the PR is practically feature complete, the main missing thing now is structural join/meet for partially overlapping protocols. For example, sometimes an explicit annotation is needed:

class A(Protocol):
    one: int
    def __len__(self) -> int: ...

class B(Protocol):
    another: int
    def __len__(self) -> int: ...

a: A
b: B
reveal_type([a, b])  # Revealed type is 'builtins.list[builtins.object*]'

y: List[Sized] = [a, b]  # This works OK

The reason for this is that I decided to re-use Instances for protocols, and real structural join/meet would require creating a "fake" TypeInfo to describe the resulting protocol. However, I have encountered problems with serialization/de-serialization of "fake" TypeInfos and postponed this feature (as proposed by Jukka). On the other hand, reusing Instances for protocols allowed to implement this in a very simple way. The PR is very small and 2/3 of it are extensive tests.

There is something I encountered while working on this PR. It looks like structural subtyping is significantly slower than nominal. This is why I also implement "subtype cache" for Instances, so that there is a net win in speed of few percent (even when I locally updated typeshed replacing all ABC in typing with protocols).

In terms of work-flow, I would be happy if people will try this PR and give also feedback on behaviour, not just a code review. I will soon submit a PR to typeshed with updates for typing stubs, and a PR with runtime implementation. As well, a PR to PEPs repo with an updated draft that takes into account comments that appeared so far.

@JukkaL @ambv @gvanrossum

@JukkaL Thanks! I think I have implemented all comments so far. The only question left is how precise we should be with variances in typing-full.pyi fixture? If you think it makes sense to match typeshed, then I will also update this.

Another batch of test case reviews.

Maybe add test where a class subtly does not implement a recursive protocol. For example, the return type of next() is not compatible but otherwise things are fine.

Again, consider testing more incompatible cases, such as attr2 existing but with an incompatible list item type.

Overloaded method implementation should probably be optional in a protocol. If there is no implementation, the method would be treated as abstract.

Test that a class with an overloaded method with an identical signature is treated as compatible. Test that a class with an invalid overloaded method is treated as incompatible.

Also test the trivial case of [x, x] or similar.

Also call with a list argument?

Use # E: ... comment?

Do you test anywhere what happens if a class has method and the protocol has an attribute/property, and vice versa?

It looks like no, I added two tests.

Typo: Ipmlicit

I don't remember if I saw a test case that would have a related case where the protocol is non-generic and a non-generic class has a generic method. Say, protocol has signature int -> int and class has signature T -> T (but T has method scope).

I have one, but only with self types, I added two tests with just "usual" generic methods.

Another batch of comments. Hopefully the next batch will be the last one.

Create issue about variance checks for recursive protocols? It seems that a set of mutually recursive protocols should be checked as a unit, since the variance in one protocol can affect variance in another protocol.

Should we erase the actual type as well?

I am not sure, but in view of #3656 I am reluctant to increase similarity of anything to protocols.

Add something about the correctness of this. Maybe mention that this is a standard way to perform structural subtype checks.

Add comment about why this is safe.

This and the previous section of code look very similar. Can you merge them to avoid code duplication?

Add comment. Some edge cases to discuss: What about inherited protocol members? What about members defined in object?

The first sentence is not descriptive enough. This is a pretty tricky concept so please explain it in more detail and maybe give an example.

Explain what goes into the stack and why we need it.

Typo: 'assumig'.

Document cache and cache_proper. Explain why we have them.

This concludes my review. As I mentioned earlier, fixing most issues now is optional -- there wasn't anything super critical. They can be fixed after this has been merged. If you leave things out, create issues which mention comments that haven't been followed through yet to more easily track what is not done yet.

Do we need to propagate ignore_declared_variance in all recursive calls?

Ideally, yes, but there are too many places, this is related to #3828, I will add a TODO item here.

Would be better style to use a TypeInfo method such as record_subtype_cache_entry(left, right) instead of directly accessing cache.

Also, should we only add to cache if no subtype flags such as ignore_pos_arg_names are being used?

I am not super sure, but I think it is OK to always add them, since these are only for Instances and they (at least currently) are always used with ignore_pos_arg_names.

Would be better style to use a method such as if right.type.is_cached_subtype_check(left, right):.

Typo: argiment

What about changing allow_any to proper_subtype (and negate the value) or similar for consistency?

Again, it looks like this should be in messages.py (can still be a top-level function) and should include protocol in the name for clarity.

Like above, move to messages.py and include 'protocol' in the function name.

What's this change?

This is not only related to protocols, but it affected several tests. This is actually a step in the direction of #3827. The logic is following, if I erase both item and s, then there will be "too many" Anys there, and is_proper_subtype will return False, while at runtime isinstance() will actually return True for protocols. Maybe this is not 100% ideal, but it fixed several test failures without causing new failures in existing tests.

More generally, I think we could reconsider use of is_proper_subtype(erased_t, erased_s) for isinstance() with nominal types, maybe there could be a better concept, something like is_runtime_subtype? I don't think we need a separate issue for this, this is basically just an extended discussion for #3827

Yeah, this seems okay. Maybe add a reference to #3827 here?

OK, I will add a comment.

If you add helper methods for accessing the cache, use them here.

I haven't reviewed this file but I can do it later (even after the PR has been landed) since this looks pretty straightforward.

Before protocols are usable, Protocol needs to be added to typing_extensions (this can happen after this PR has been merged).

@JukkaL

Protocol needs to be added to typing_extensions

Yes, I will do this soon.

@JukkaL It looks like all the comments are implemented now (or corresponding issues filed).

Thanks for review once more!

This looks good now! Thanks for persevering with the very long review process!

Here are some important follow-up tasks:

Add implementation of Protocol to typing_extensions (or if @gvanrossum feels that it's too early, it can go to mypy_extensions) and release a version of typing_extensions with Protocol.
Add definition of Protocol to typeshed (but don't turn existing ABCs in typeshed into protocols yet).

After the above steps user-defined protocols should work.

Changing typing ABCs to be protocols can happen later -- potentially we'll have a mypy release with only support for user-defined protocols first. It's still not clear how we'll move forward with the bigger typeshed change.

Add implementation of Protocol to typing_extensions

I will hopefully make PRs for typing_extensions (implementation and stubs) tomorrow. I think it is OK to go straight with typing_extensions since @vlasovskikh (and maybe other) is already interested in this.

By the way, @matthiaskramm what do you (i.e. pytype team) think about protocols and making things like Iterable etc. protocols in typeshed?

Woot! Congrats and thanks everyone.

Thanks!

@ilevkivskyi +1 for putting Protocol into typing_extensions.

Ivan Levkivskyi and others added 30 commits March 26, 2017 14:20

Recognize protocols in semanal.py

884481d

Add issubtype and basic constraints

c19b6d4

Merge remote-tracking branch 'upstream/master' into protocols

83c1579

Remove debugging code

b01f5b0

Minor fixes: semanal, constrains, lib-stub; add few tests

16901fc

Allow mypy_extensions.runtime, revert change to test name.

c8c1247

Fix typo

f430c65

Merge remote-tracking branch 'upstream/master' into protocols

beea7d2

Enhance constraints, add join (meet seems to be already OK); minor fixes

260237a

Merge remote-tracking branch 'upstream/master' into protocols

5414eaf

Prohibit protocol instatiation except Type[]; start working on @runtime

7f8a514

Add some tests

92b71a2

Restore docstring formstting

83085c7

Merge remote-tracking branch 'upstream/master' into protocols

27b8570

Fix serialization; add few more tests

41179c2

Add notes for missing protocol members in assignment

5171844

Add notes for missing members also for arguments and return types

db123a3

Prohibit definition via self.x in protocol classes

31bf16c

Merge remote-tracking branch 'upstream/master' into protocols

03b5d72

Make more things in fixtures structural subtypes

65a8546

Structural support for __call__; add more tests

2768d76

Fix lint and tests

93beb75

Beautify error notes

5135fd9

Bind self to potential subtype; add tests for structural self-types

fdeb89b

One more test

1fbcb4a

Move global data to nodes.TypeInfo

d012e38

Add comments and docstrings; recognize and treat abstract variables

801770d

Fix tests and lint

a625f37

Merge remote-tracking branch 'upstream/master' into protocols

fb6b1ad

Add two more tests for recursive

adb68eb

Address second part of recent comments

40d635f

ilevkivskyi mentioned this pull request Aug 12, 2017

Enforce compatible __init__ in structural subtypes #3823

Open

JukkaL reviewed Aug 14, 2017

View reviewed changes

ilevkivskyi mentioned this pull request Aug 14, 2017

Variance of generics in typing-full fixture #3826

Open

First part of todays comments

19073e5

ilevkivskyi mentioned this pull request Aug 14, 2017

Safer treatment of @runtime protocols in isinstance() #3827

Closed

This was referenced Aug 15, 2017

Implement variance checks for recursive protocols. #3828

Open

Improve type inference for recursive protocols #3829

Open

Second part of yesterdays comments

50d98c0

JukkaL mentioned this pull request Aug 15, 2017

Fix handling of non-method callable attribute #3227

Closed

ilevkivskyi mentioned this pull request Aug 15, 2017

Implement better join and meet for protocols #3830

Closed

JukkaL mentioned this pull request Aug 15, 2017

Use object as the fallback for various Callable types #3831

Closed

ilevkivskyi mentioned this pull request Aug 15, 2017

Refactor common code in analyze_member_access and find_member #3832

Closed

ilevkivskyi added 2 commits August 15, 2017 20:09

Last part of review comments

9411255

One last missing comment

f1c915e

JukkaL approved these changes Aug 16, 2017

View reviewed changes

JukkaL merged commit c55e48b into python:master Aug 16, 2017

JukkaL mentioned this pull request Aug 16, 2017

User-defined protocols #3838

Closed

evhub mentioned this pull request Sep 6, 2017

Mypy error evhub/coconut#317

Closed

+                  def f(self, x: int) -> int: pass
+                  @overload
+                  def f(self, x: str) -> str: pass
+                  def f(self, x): pass

+                          down_args = [UninhabitedType() if i == j else AnyType(TypeOfAny.special_form)
+                                       for j, _ in enumerate(tvars)]
+                          up, down = Instance(info, up_args), Instance(info, down_args)
+                          # TODO: add advanced variance checks for recursive protocols

                           # subtyping algorithm if type promotions are possible (e.g., int vs. float).
                           if formal.type in actual.type.mro:
                               return 2
+                          elif formal.type.is_protocol and is_subtype(actual, erasetype.erase_type(formal)):

                               return res
+                          if (template.type.is_protocol and self.direction == SUPERTYPE_OF and
+                                  # We avoid infinite recursion for structural subtypes by checking
+                                  # whether this type already appeared in the inference chain.

+                              for member in template.type.protocol_members:
+                                  inst = mypy.subtypes.find_member(member, instance, original_actual)
+                                  temp = mypy.subtypes.find_member(member, template, original_actual)
+                                  assert inst is not None and temp is not None

+              t: P1
+              t = A() # OK
+              t = B() # E: Incompatible types in assignment (expression has type "B", variable has type "P1")

+                  def f(self, x: int) -> None:
+                      pass
+              x: P = C()


		l = [x, y]

		reveal_type(l) # E: Revealed type is 'builtins.list[__main__.P*]'

+              def bar(a: Sized) -> int:
+                  return a.__len__()
+              bar(Foo())

+              [builtins fixtures/isinstancelist.pyi]
+              [out]
+              main:11: error: Argument 1 to "foo" has incompatible type "str"; expected "SupportsInt"

+                  @property
+                  def attr2(self) -> int: pass
+              x: P = C() # E: Incompatible types in assignment (expression has type "C", variable has type "P") \

+              fun_p(C())  # OK
+              [builtins fixtures/list.pyi]
+              [case testIpmlicitTypesInProtocols]

+              class P(Protocol[T]): # E: Invariant type variable 'T' used in protocol where covariant one is expected
+                  attr: int
+              [case testGenericProtocolsInference1]

+                                      res.extend(infer_constraints(temp, inst, neg_op(self.direction)))
+                              template.type.inferring.pop()
+                              return res
+                          elif (instance.type.is_protocol and self.direction == SUBTYPE_OF and

+                  is_protocol = False                    # Is this a protocol class?
+                  runtime_protocol = False               # Does this protocol support isinstance checks?
                   abstract_attributes = None  # type: List[str]
+                  protocol_members = None  # type: List[str]

		stack.pop()


		def is_protocol_implementation(left: Instance, right: Instance, allow_any: bool = True) -> bool:

		return missing


		def get_conflict_types(left: Instance, right: Instance) -> List[Tuple[str, Type, Type]]:

		return conflicts


		def get_bad_flags(left: Instance, right: Instance) -> List[Tuple[str, Set[int], Set[int]]]:

+                  # during the type checking phase.
+                  assuming = None  # type: List[Tuple[mypy.types.Instance, mypy.types.Instance]]
+                  assuming_proper = None  # type: List[Tuple[mypy.types.Instance, mypy.types.Instance]]
+                  # Ditto for temporary stack of recursive constraint inference.

+                  inferring = None  # type: List[mypy.types.Instance]
+                  cache = None  # type: Set[Tuple[mypy.types.Type, mypy.types.Type]]
+                  cache_proper = None  # type: Set[Tuple[mypy.types.Type, mypy.types.Type]]
+                  # 'inferring' and 'assumig' can't be also made sets, since we need to use

                               for base in left.type.mro:
                                   if base._promote and is_subtype(
                                           base._promote, self.right, self.check_type_parameter,
                                           ignore_pos_arg_names=self.ignore_pos_arg_names):

                           return is_subtype(left, right.fallback)
                       if isinstance(right, Instance):
-                          # NOTO: left.type.mro may be None in quick mode if there
+                          if (left, right) in right.type.cache:

+                          # nominal subtyping currently ignores '__init__' and '__new__' signatures
+                          if member in ('__init__', '__new__'):
+                              continue
+                          # The third argiment below indicates to what self type is bound.

                       new_items = [item for item in t.relevant_items()
-                                   if (not is_proper_subtype(erase_type(item), erased_s)
+                                   if (not (is_proper_subtype(erase_type(item), erased_s) or
+                                            is_proper_subtype(item, erased_s))

                   def visit_instance(self, left: Instance) -> bool:
                       right = self.right
                       if isinstance(right, Instance):
+                          if (left, right) in right.type.cache_proper:

                   def accept(self, visitor: 'TypeVisitor[T]') -> T:
                       return visitor.visit_unbound_type(self)
+                  def __hash__(self) -> int:

Protocols by ilevkivskyi · Pull Request #3132 · python/mypy · GitHub

Uh oh!

Uh oh!

Protocols - #3132

Protocols#3132JukkaL merged 152 commits intopython:masterpython/mypy:masterfrom ilevkivskyi:protocolsilevkivskyi/mypy:protocolsCopy head branch name to clipboard

Conversation

ilevkivskyi commented Apr 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilevkivskyi commented Aug 12, 2017

Uh oh!

JukkaL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JukkaL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JukkaL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Protocols#3132
JukkaL merged 152 commits into
python:masterfrom
ilevkivskyi:protocols

ilevkivskyi commented Apr 4, 2017 •
edited
Loading