Adapt and clarify tests and docs around explicitly annotated receiver parameters by sharkdp · Pull Request #2363 · python/typing

sharkdp · 2026-09-22T15:01:39Z

The conformance tests currently include a test in protocols_generic.py which wants type checkers to accept assignability of ConcreteHasParent to the HasParent protocol:

T = TypeVar("T")

class HasParent(Protocol):
    def get_parent(self: T) -> T:
        ...

class ConcreteHasParent:
    def get_parent(self) -> Self:
        return self

However, the get_parent method of the protocol seems to be more general than what ConcreteHasParent implements. The T type variable is unbounded, while the implicit Self annotation on the unannotated self received of ConcreteHasParent.get_parent should be bounded by ConcreteHasParent. The protocol should theoretically support calls like the following, through type[HasParent], whereas ConcreteHasParent would not:

def f(h: type[HasParent]):
    h.get_parent("whatever")

The spec includes a paragraph that states:

Note that some type checkers may apply restrictions on [the use of an explicitly annotated receiver parameter], such as requiring an appropriate upper bound for the type variable used

On the other hand, no type checker seems to do so (pyright assumes an implicit upper bound, but that's not supported by that statement in the spec). Given that, I think it would be best to just define an upper bound for the type variable here.

I also fixed that problem (using an unbounded type variable for as an annotation on a receiver parameter) elsewhere in the tests and the documentation, often by using Self instead, if it seemed like the test was about something unrelated.

sharkdp · 2026-09-22T15:05:45Z

 conformance_automated = "Fail"
 conformant = "Partial"
 notes = """
-Only partially supports `@property` protocol members.


I am a ty maintainer. This description was outdated.

I am not making this change to make ty look better on the conformance tests. By disentangling the different aspects of the previous ConcreteHasProperty* tests, we just isolate the deficiency of ty more clearly.

sharkdp · 2026-09-22T15:08:09Z

 conformant = "Partial"
 notes = """
-Only partially supports `@property` protocol members.
+Does not reject incompatible non-generic implementations of generic protocol methods.


Cross reference: astral-sh/ty#2323

I think astral-sh/ty#2924 is the better reference here; astral-sh/ty#2323 is specifically discussing unsoundness of Self in protocols, which the spec actually mandates should be allowed. (Though it does get a bit muddied in the comments, since in ty Self is simply a bounded typevar.)

sharkdp · 2026-09-22T15:12:08Z

-class HasPropertyProto(Protocol):
+class HasParentProperty(Protocol):
    @property
-    def f(self: T) -> T:
-        ...
-
-    def m(self, item: T, callback: Callable[[T], str]) -> str:
+    def parent(self) -> Self:
        ...


The existing tests here were convoluted (the ConcreteHasProperty* assignments failed for multiple reasons). It looks to me like they wanted to tests generic properties and generic methods at the same time and failed to do so. Looking back at the history, I can see that the assertion on the hp2: HasPropertyProto = ConcreteHasProperty2() line has changed from "OK" to "E", further illustrating that it's not really clear what was being tested there.

In the new version, I tried to stay close to the original (I can deviate further if that is desirable for educational reasons, for example), while disentangling the generic-property-related tests from the generic-method-related tests.

sharkdp · 2026-09-22T15:13:24Z

-class ConcreteHasProperty1:
+class ConcreteParentProperty:
    @property
-    def f(self: T) -> T:


If we really want to retain a test with a property that is generic over an unbounded type variable, I can add that again. Otherwise, it seems like using Self is sufficient here (which also makes that property generic)?

sharkdp · 2026-09-22T15:26:55Z

 Yes::

-    _T = TypeVar("_T")
+    _T = TypeVar("_T", bound="Foo")


I can omit this, but it seemed sensible to add.

Shouldn't this example be using Self?

sharkdp · 2026-09-22T15:27:04Z


  class Copyable(Protocol):
      def copy[C: Copyable](self: C) -> C:
+          ...


Drive-by fix.

sharkdp · 2026-09-22T15:28:48Z

+          ...

  class One:
-      def copy(self) -> 'One':


This seems like it violates the generic type of Copyable.copy, since there's no "you can use Foo instead of Self" exception for arbitrary type variables in the spec.

… parameters

carljm

Looks good to me, but I'll wait for another reviewer before merging.

carljm · 2026-09-22T15:54:15Z


  class One:
-      def copy(self) -> 'One':
+      def copy(self) -> Self:


What's the rationale for this change? I think the original -> 'One' is already allowed, per the spec: the generics spec treats protocol Self as equivalent to a protocol-bounded TypeVar and allows implementations that return their concrete class. The new version with -> Self is also valid, but makes One and Other demonstrate closer-to-equivalent annotations instead of two distinct supported cases.

See my inline annotation on the negative part of the diff:

This seems like it violates the generic type of Copyable.copy, since there's no "you can use Foo instead of Self" exception for arbitrary type variables in the spec.

You wrote:

the generics spec treats protocol Self as equivalent to a protocol-bounded TypeVar and allows implementations that return their concrete class.

The actual protocol in question here,Copyable, does not use Self. It uses a (properly bounded) typevar tough, so if we combine your quote of the spec with "Self is valid within Protocols [it] is treated equivalently to [bounded typevars]", then maybe one could conclude that returning One is also valid here.

It didn't seem clear from the spec to me, which is why I replaced it One with Self, but I can revert that.

I reverted this change.

I agree this is really unclear in the spec. It probably deserves a separate PR that is not just a conformance suite clarification, but an actual spec clarification/change with accompanying conformance suite updates.

(Sorry I missed your comment; I've found that much easier to do accidentally now that GitHub collapses them by default.)

carljm · 2026-09-22T15:57:44Z

 conformant = "Partial"
 notes = """
-Only partially supports `@property` protocol members.
+Does not reject incompatible non-generic implementations of generic protocol methods.


I think astral-sh/ty#2924 is the better reference here; astral-sh/ty#2323 is specifically discussing unsoundness of Self in protocols, which the spec actually mandates should be allowed. (Though it does get a bit muddied in the comments, since in ty Self is simply a bounded typevar.)

JelleZijlstra · 2026-09-22T20:02:37Z

 Yes::

-    _T = TypeVar("_T")
+    _T = TypeVar("_T", bound="Foo")


Shouldn't this example be using Self?

JelleZijlstra · 2026-09-22T20:03:55Z

+
+class InvalidParentProperty:
+    @property
+    def parent(self) -> int:


a more interesting example might be having this return HasParentProperty

sharkdp force-pushed the explicitly-annotated-receivers branch from 807a7e6 to 622f477 Compare September 22, 2026 15:25

sharkdp commented Sep 22, 2026

View reviewed changes

Adapt and clarify tests and docs around explicitly annotated receiver…

c34f258

… parameters

sharkdp force-pushed the explicitly-annotated-receivers branch from 622f477 to c34f258 Compare September 22, 2026 15:34

sharkdp marked this pull request as ready for review September 22, 2026 15:37

carljm approved these changes Sep 22, 2026

View reviewed changes

Revert 'One' -> Self change

8f9607e

JelleZijlstra reviewed Sep 22, 2026

View reviewed changes

Conversation

sharkdp commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

sharkdp Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

carljm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharkdp Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carljm Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sharkdp commented Sep 22, 2026 •

edited

Loading

sharkdp Sep 22, 2026 •

edited

Loading

sharkdp Sep 22, 2026 •

edited

Loading

carljm Sep 22, 2026 •

edited

Loading