Skip to content
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

Add typing.get_protocol_members and typing.is_protocol #104873

Closed
JelleZijlstra opened this issue May 24, 2023 · 12 comments
Closed

Add typing.get_protocol_members and typing.is_protocol #104873

JelleZijlstra opened this issue May 24, 2023 · 12 comments
Labels
topic-typing type-feature A feature request or enhancement

Comments

@JelleZijlstra
Copy link
Member

JelleZijlstra commented May 24, 2023

#103160 added an attribute __protocol_attrs__ that holds the names of all protocol attributes:

>>> from typing import Protocol
>>> class X(Protocol):
...     a: int
...     def b(self) -> str: pass
... 
>>> X.__protocol_attrs__
{'b', 'a'}

This is useful for code that needs to extract the names included in a Protocol at runtime. Previously, this was quite difficult, as you had to look at the class's __dict__ and __annotations__ directly and exclude a long list of internal attributes (e.g. https://github.com/quora/pyanalyze/blob/bd7f520adc2d8b098be657dfa514d1433bea3b0c/pyanalyze/checker.py#L428).

However, currently __protocol_attrs__ is an undocumented private attribute. I think we should either document it or add an introspection helper like typing.get_protocol_attrs() that exposes it.

Linked PRs

@JelleZijlstra JelleZijlstra added type-feature A feature request or enhancement topic-typing labels May 24, 2023
@AlexWaygood
Copy link
Member

AlexWaygood commented May 24, 2023

The worry about documenting it is that people might start monkey-patching it, which I don't want to encourage :)

We could mitigate that by making it a read-only property (and by making it a frozenset instead of a mutable set). But making it a property might introduce some performance overhead, and the whole reason we added this attribute was to address a performance issue. Having a dunder attribute also wouldn't work very well with static type checkers -- because of how special Protocol is in typeshed's stubs, we wouldn't be able to add a stub for the attribute in typeshed, so mypy et al. would complain every time a user tried to access the dunder on a protocol class.

So, I guess I vote for a typing.get_protocol_attrs() "getter" function! It feels like it avoids all the problems I mentioned above.

@JelleZijlstra
Copy link
Member Author

Yes, I think a getter function makes sense here. I was just thinking about this while writing a patch for #104874, where I do think it makes sense to simply document the dunder attribute. The difference there is that the NewType class itself is simple and the attribute isn't used at runtime. Protocols, on the other hand, have to deal with complex subclassing relationships and it's plausible we'll want to make changes to the implementation in the future that change how the dunder works.

@AlexWaygood
Copy link
Member

Yes, I think a getter function makes sense here. I was just thinking about this while writing a patch for #104874, where I do think it makes sense to simply document the dunder attribute. The difference there is that the NewType class itself is simple and the attribute isn't used at runtime. Protocols, on the other hand, have to deal with complex subclassing relationships and it's plausible we'll want to make changes to the implementation in the future that change how the dunder works.

Agreed on all counts.

@AlexWaygood
Copy link
Member

AlexWaygood commented May 24, 2023

get_protocol_members() would probably be a better name for the function, btw. I called the attribute __protocol_attrs__ because the attribute __protocol_attrs__ is calculated by calling typing._get_protocol_attrs (which has been in typing for many years, and there's no real reason to rename). But I think __protocol_members__ would probably have been a better name for the attribute, really. I probably would have thought longer about the name if I'd planned for it to become public API ;)

JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this issue May 24, 2023
@JelleZijlstra JelleZijlstra changed the title Make __protocol_attrs__ documented and public Add typing.get_protocol_members and typing.is_protocol May 24, 2023
@JelleZijlstra
Copy link
Member Author

Based on PR review, going to also add typing.is_protocol.

@Giddius
Copy link

Giddius commented May 25, 2023

Based on PR review, going to also add typing.is_protocol.

I know that this is most relevant for typing but just as comment and not actual critique:
Almost all is_x functions that are not builtins are part of inspect and most do not use the underscore format:

  • inspect.isabstract
  • inspect.isawaitable
  • inspect.isdatadescriptor
  • inspect.iscoroutine

With asyncio.iscoroutine being in asyncio and typing.is_typeddict in typing being the exceptions.

——
This again is meant more as a comment not a critic or request.

Edit: removed word at the end that was left in by accident.

JelleZijlstra added a commit that referenced this issue Jun 14, 2023
…04878)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@AlexWaygood
Copy link
Member

Oh, I think this is done now! 🎉

@NeilGirdhar
Copy link

NeilGirdhar commented Jul 2, 2023

Just out of curiosity, but does is_protocol(x) differ from issubclass(x, Protocol) and x not in {typing_extensions.Protocol, typing.Protocol}?

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 2, 2023

Just out of curiosity, but does is_protocol(x) differ from issubclass(x, Protocol) and x not in {typing_extensions.Protocol, typing.Protocol}?

Yes — x might be a subclass of typing_extensions.Protocol, and typing.is_protocol(x) should still evaluate to True

@AlexWaygood
Copy link
Member

@NeilGirdhar the relevant commit is here if you'd like to read the source code :-) fc8037d

@JelleZijlstra
Copy link
Member Author

The more important reason is that concrete classes can be subclasses of Protocol.

In [1]: from typing import Protocol

In [2]: class Proto(Protocol):
   ...:     def f(self) -> int: ...
   ...: 

In [3]: class Concrete(Proto):
   ...:     def f(self) -> int: return 0
   ...: 

In [4]: issubclass(Concrete, Protocol)
Out[4]: True

@NeilGirdhar
Copy link

@JelleZijlstra Makes perfect sense, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-typing type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants