PEP 849: "More Expressive Type Expressions" - #5132
ImogenBits wants to merge 25 commits into
Conversation
|
The following commit authors need to sign the Contributor License Agreement: |
Documentation build overview
5 files changed ·
|
| PEP: 849 | ||
| Title: More Expressive Type Expressions | ||
| Author: Imogen Hergeth <python at imogen.tech> | ||
| Sponsor: Jelle Zijlstra <jelle.zijlstra at gmail.com> |
There was a problem hiding this comment.
Confirming my sponsorship
| this a worthwhile trade off. The specification of the proposed format also is open | ||
| enough that many optimizations are possible should they be deemed necessary in | ||
| the future. For many users, this proposal will even be a slight performance | ||
| increase since their code never evaluates any annotate functions. |
There was a problem hiding this comment.
What about the effect on e.g. dataclasses, which need to evaluate their annotations? Will this make dataclass creation slower?
There was a problem hiding this comment.
I've added a paragraph explaining the impact of this PEP on these kinds of introspection tools.
| *********************** | ||
|
|
||
| Since this PEP only adds new functionality, there are no direct backwards | ||
| compatibility concerns. However, we also want to point out that future additions |
There was a problem hiding this comment.
Deprecating get_type_hints is a pretty big compatibility concern
There was a problem hiding this comment.
Do you mean that I should add a part in the backwards compatibility section talking about this or drop the deprecation entirely? I don't have any strong opinions either way, I just assumed that since get_type_hints would be largely useless it is better to deprecate it than have a somewhat broken helper function in the standard library.
There was a problem hiding this comment.
I think deprecating it is fine, but it deserves explicit discussion here.
| Storing Annotation Source Code | ||
| ============================== | ||
|
|
||
| Instead of storing binary data that defines the annotations' ASTs an alternative |
There was a problem hiding this comment.
The current approach means that the STRING format will still not faithfully represent the original string though, right? I think that's a minor issue but worth noting.
There was a problem hiding this comment.
I've thought about this but I'm not sure what the best approach is. The AST data can be used to recreate the actual source code for the STRING format (same with FORWARDREF). We wouldn't even have to add much since the current approach already is to construct an AST and then unparse it. We could either make the annotate functions directly do that to support the format or implement another case in get_annotations. I haven't added that to the PEP so far just because it isn't directly related to the main idea behind it, but if it would be better to include it I'd be happy to.
There was a problem hiding this comment.
Oh, I was thinking of the fact that we're losing formatting details (e.g. whitespace), which I assume would still be the case with your proposal.
I definitely think that we should implement STRING on top of AST now. It would be much simpler (essentially just calling ast.unparse) and work better in edge cases.
There was a problem hiding this comment.
That's a good point, I've now explicitly pointed this out in the PEP.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
2 participants
Basic requirements (all PEP Types)
pep-NNNN.rst), PR title (PEP 123: <Title of PEP>) andPEPheaderuvx pepotron next(orpipx install pepotronthenpep next)AuthororSponsor, and formally confirmed their approvalAuthor,Status(Draft),TypeandCreatedheaders filled out correctlyPEP-Delegate,Topic,RequiresandReplacesheaders completed if appropriate.github/CODEOWNERSfor the PEPStandards Track requirements
Python-Versionset to valid (pre-beta) future Python version, if relevantDiscussions-ToandPost-HistoryI've used 849 since this is a continuation of PEPs 649 and 749 and it's almost the next available number. I'm happy to change it to whatever is next (currently 848) if that is more appropriate.