gh-130881: Handle conditionally defined annotations

[JelleZijlstra]

• Issue: conditional blocks in class definitions seem to be evaluating types even when these conditionals are false #130881

[JelleZijlstra]

Thanks @picnixz @tomasr8 for the reviews, please take another look!

[picnixz]

I won't force you to follow PEP-7 as the file has a mix of everything which makes it very hard to choose a convention. You can also ignore my suggestion in symtable.c if you think it's not necessary and if the surrounding code is using the same writing style.

[picnixz]

I think I reviewed the changes, though I'm not entirely sure about the tests themselves so another eye is welcome

[tomasr8]

Once this is merged, should the text in PEP 649 be updated to reflect this change?

Code that sets annotations on module or class attributes from inside any kind of flow control statement. It’s currently possible to set module and class attributes with annotations inside an if or try statement, and it works as one would expect. It’s untenable to support this behavior when this PEP is active.

[tomasr8]

Just some comments/questions for the tests, otherwise looks good :)

[JelleZijlstra]

Once this is merged, should the text in PEP 649 be updated to reflect this change?

Code that sets annotations on module or class attributes from inside any kind of flow control statement. It’s currently possible to set module and class attributes with annotations inside an if or try statement, and it works as one would expect. It’s untenable to support this behavior when this PEP is active.

I don't own PEP 649 so I can't update it. Could add it to PEP 749 but I already submitted it to the SC.

[tomasr8]

Thanks! Looks good to me :)

[tomasr8]

Once this is merged, should the text in PEP 649 be updated to reflect this change?

Code that sets annotations on module or class attributes from inside any kind of flow control statement. It’s currently possible to set module and class attributes with annotations inside an if or try statement, and it works as one would expect. It’s untenable to support this behavior when this PEP is active.

I don't own PEP 649 so I can't update it. Could add it to PEP 749 but I already submitted it to the SC.

Larry is the owner. @larryhastings should the PEP be updated?

[larryhastings]

Accepted PEPs are historical documents; as a rule I'm averse to changing them after they're accepted. PEP 749 is the perfect place to document this change to what's specified in 649.

[picnixz]

Two nits I missed but otherwise, looks good to me

[SnoopJ]

EDIT: I was reading PEP 749 too quickly and missed that there is a section on conditionally-defined annotations (because it does not mention the new dunder by name). I guess the PEP is pretty descriptive, but my question about adding doc of the dunder still stands, I think. I had to consult a blame for compile.c to find the PR that introduced the new identifier.

@JelleZijlstra I just stumbled across this functionality and was surprised that I had to dive into the codebase's history to find out what it was all about. Looks like the window for including this info in PEP 749 has passed, but should there perhaps be an issue for adding information about this feature to the Data Model docs?

[JelleZijlstra]

The dunder is private. Do we have documentation for other purely internal dunders?

[SnoopJ]

Do we have documentation for other purely internal dunders?

There is a section on internal types which might count towards answering that question? Downstreams I think would have fewer reasons for looking at __conditional_annotations__ (since most interested consumers would "just" consult the result of calling __annotate__ I understand the feature right?), and the storage isn't really a 'type'. module.__cached__ is perhaps also in the same vein.

I can see the argument for "nobody should need to know about this" but figured I'd ask on behalf of anyone else who was surprised to discover the dunder's existence and then have trouble dredging up more information about what it's for.

[JelleZijlstra]

If you'd like this documented please open a new issue. But note that several other private class attributes, such as __basicsize__, are also undocumented.