Change the typing spec around string references - #2144
Conversation
|
@JelleZijlstra Could you please pre-review this? What do you think about this spec change? |
|
I think I have integrated all the changes. Is it time to open an issue on the Typing Council’s issue tracker asking for a decision? |
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
carljm
left a comment
There was a problem hiding this comment.
One wording nit, one formatting nit, and one conformance suite nit :) But overall this looks great to me.
rchen152
left a comment
There was a problem hiding this comment.
Looks good to me - much more consistent and clearly specified than before
carljm
left a comment
There was a problem hiding this comment.
This looks good to me. Thanks @davidhalter for getting this clarified.
|
I have integrated all of Carl's suggestions. I will update the conformance tests as soon as the typing council approves this change. If I update it now we probably just run into merge conflicts, since especially pyrefly changes a lot. @carljm Please let me know if you think something needs more work. |
|
Hi, can someone explain the intent of this change to me? Given, under python 3.14: does this change propose that it would be impossible for |
|
Yes |
|
are you going to change the behavior of edit: the pep is pep-749 |
|
existing libraries which use this pattern will have to use: |
|
this would be on top of this breaking change in 3.13 -> 3.14 |
|
I went to where I knew I'd find examples of this, in |
jorenham
left a comment
There was a problem hiding this comment.
When reading this at first I was a bit worried that this would cause issues for NumPy, where builtin shadowing (e.g. numpy.bool) and class shadowing (numpy.dtype / numpy.ndarray.dtype) are pretty common things. However, this was recently fixed by @JelleZijlstra in numpy/numpy#31951, so that's no longer an issue.
Anyway, this change reduces ambiguity while improving precision in a directly testable way. The main selling point of string annotations are forward references, so it seems natural for it to shadow "as above, so below". The other explicit "no special casing" vibes I'm getting from this I also really like.
Not "on top of" - it's the same change. What this PR does is reflect that very same change in runtime semantics of annotations in the specified type checker behavior, so that the two are consistent.
I think this is a really good question, and personally I would be open to specifying a more gradual transition path here, even though it would mean type checkers have to maintain more different behaviors at once. Specifically, we could say that stringified annotations (whether explicitly strings, or implicitly stringified via |
|
I was struggling to wrap my head around the matrix of runtime behaviors involved here, so I put together a little table for myself. Copying it below in case it helps anyone else (all code snippets are in the body of a class
@carljm If I understand correctly, you're proposing that all stringified annotations be resolved exactly as they are at runtime, while non-string annotations are resolved as they would be in 3.14? If so, then at least for the above examples, that seems to simplify down to "match 3.14 runtime behavior exactly, even where it's conceptually inconsistent," which I can get behind, even though it's a little unsatisfying. |
|
That table is really useful. |
|
I'd rather not rely on the behavior of |
|
I fully agree, |
|
Is there a better reference than To be clear, I think that if we were starting from a clean slate, a simple and consistent interpretation of type annotations as proposed here would be ideal. But we're not, so I'm trying to understand what a compromise that still moves us in the right direction would look like. |
Hmm this is also news to me. Then in that case I agree that we shouldn't build upon unstable grounds. But at the same time I also think it's a good idea to minimize the differences between the static- and runtime-typing worlds. |
|
The non-buggy modern alternative is |
Ah perfect; that sounds like a good alternative to |
|
Generally the reference should be what happens if you don't stringify the annotation in 3.14+. Annotations are now always lazily evaluated. |
|
Perfect, thanks. I retried using If the intended reference is the non-stringified 3.14 results, then this PR matches the intended runtime behavior in 3.14 onward, which is great.
[1] using this code, if anyone wants to check my homework ;) I added |
|
@rchen152 Yes, I think that's a good summary of my suggestion above: to match the runtime behavior of 3.14 exactly, assuming that The alternative proposal on the table we could call "simple" -- that proposal is that all annotations (including stringified ones) should be resolved the way they would resolve at runtime if they were non-stringified annotations on 3.14+. To the extent that stringified annotations (mostly) eventually go away, the two proposals will converge, since they differ only in the handling of stringified annotations. The advantage of "simple" is that there is a single consistent model of name resolution for type checkers to implement. The disadvantages are:
The "compat" model preserves backwards-compatibility and full consistency with runtime behavior, allowing users to migrate to the new name-resolution semantics on their own schedule (by moving from stringified annotations to non-stringified PEP 647/747 in 3.14+). The cost is more complexity in type checker implementations. It increasingly seems to me that "compat" is the choice that better serves users of the type system. (There is a third possible choice, which is to fully embrace the backwards-incompatible change by also changing the runtime behavior of |
I added this after the discussion here: https://discuss.python.org/t/annotation-string-references-in-class-scope-in-conformance-tests/105439
I'm not 100% sure about the wording, but I hope the direction is fine. I would like to gather some feedback before presenting this to the typing council.
Please also merge #2139 before this pull request. Otherwise it will be very hard to update Zuban's conformance test results in this pull request.