Repository navigation
conditional blocks in class definitions seem to be evaluating types even when these conditionals are false #130881
Description
Bug report
Bug description:
using a conditional in a class definition like if TYPE_CHECKING where TYPE_CHECKING is False, or any kind of false conditional, seems to be ignored when types are evaluated under non-future annotations mode:
class MyClass:
somevalue: str
if False:
someothervalue: int
assert MyClass.__annotations__ == {"somevalue": str}, f"MyClass.__annotations__ == {MyClass.__annotations__}"this seems to be something that might have been done with some intention, which is extremely troubling as this would be a huge showstopper for SQLAlchemy if these conditionals are no longer honored at runtime. A similar example
from typing import TYPE_CHECKING
if TYPE_CHECKING:
# is not evaluated under any python at runtime
from some_module import SpecialType
class MyClass:
somevalue: str
if TYPE_CHECKING:
# **is** evaluated under python 3.14.0a5, fails
someothervalue: SpecialType
assert MyClass.__annotations__ == {"somevalue": str}, f"MyClass.__annotations__ == {MyClass.__annotations__}"calling upon __annotations__ seems to be the trigger that makes the above fail:
Traceback (most recent call last):
File "/home/classic/dev/sqlalchemy/test4.py", line 14, in <module>
assert MyClass.__annotations__ == {"somevalue": str}, f"MyClass.__annotations__ == {MyClass.__annotations__}"
^^^^^^^^^^^^^^^^^^^^^^^
File "/home/classic/dev/sqlalchemy/test4.py", line 12, in __annotate__
someothervalue: SpecialType
^^^^^^^^^^^
NameError: name 'SpecialType' is not definedhowever this appears to be some very strange quantum-physics type of evaluation that isn't actually running Python fully; below, the "someothervalue" type is evaluted, but not the value function assigned!
from typing import TYPE_CHECKING
def do_my_thing() -> int:
print("HEY!")
assert False
return 3 + 4
class MyClass:
somevalue: str
if TYPE_CHECKING:
someothervalue: int = do_my_thing()
assert MyClass.__annotations__ == {"somevalue": str}, f"MyClass.__annotations__ == {MyClass.__annotations__}"This is an enormous and strange behavioral change that is really going to make things extremely difficult for SQLAlchemy, starting with we will have to rewrite thousands (OK, it turned out to be "dozens") of lines of test suite code into a much bigger exploded form and also a lot of end-user use cases we've defined using TYPE_CHECKING blocks aren't going to work anymore, it's not clear how much further this change will go.
I suppose if the whole thing boils down to what __annotations__ does, and we can call upon something like __annotations_but_not_false_blocks__, that could save us, but overall I'm really hoping this is just a bad dream that we can wake up from.
CPython versions tested on:
3.14
Operating systems tested on:
Linux
Linked PRs
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
zzzeek commentedon Mar 5, 2025
so I'm reading https://peps.python.org/pep-0749/ now which seems to be where we're going to have to go
zzzeek commentedon Mar 5, 2025
if I can get some guidance on a function in
annotationlibthat will give me annotations with some way to ignoreFalseblocks, rather than us calling__annotate__, that would be helpfulzzzeek commentedon Mar 5, 2025
this change is really quite bizarre and I continue to hope this isn't intended
under python 3.13 and below prints
{'somevalue': <class 'str'>, 'someothervalue': <class 'int'>}, under 3.14.0a5 prints{'somevalue': <class 'str'>, 'someothervalue': <class 'str'>}. if you reverse the order of the conditional, then they matchpython3.14.0a5 is literally running through code and quasi-evaluating it without paying attention to conditionals, this is really the most Perl-ish thing I've ever seen in Python by a mile
vytas7 commentedon Mar 5, 2025
On the other hand, it seems to open new possibilities to interesting experiments that were not possible before 🤔
tomasr8 commentedon Mar 5, 2025
cc @JelleZijlstra
zzzeek commentedon Mar 5, 2025
I think the answer we're going to get for all of these is that "typing is static and should not have conditionals"
I also have vague memories of this discussion re:
__annotations__running all the annotations, etc. and I probably had similar concerns at that time.this hits us in the way our test suite is written, which is one side of it, but we also have some end-user recipes where on the SQLAlchemy side we'd have to tell them to not use TYPE_CHECKING and instead some kind of sentinel value that SQLAlchemy can evaluate, so that when it gets
__annotations__from a class, we have even more contextual information such that "this particular annotation is only for typing purposes, dont evaluate it in a SQLAlchemy runtime context"CaselIT commentedon Mar 5, 2025
Personally I don't think it's correct to evaluate false branches in a way that have effect at runtime. So I hope this is treated as a bug.
The fact that you have
someothervalueis evaluated to be a string in the example mentioned here #130881 (comment) makes no logical sense.sobolevn commentedon Mar 5, 2025
Not at all! Typing is not only static, we also care about our runtime type checkers.
Regression in #130881 (comment) 100% looks like a bug to me.
zzzeek commentedon Mar 5, 2025
well while we wait in suspense, it's just this behavior is so weird that I find it hard to believe it was done by accident!
JelleZijlstra commentedon Mar 6, 2025
This is documented in PEP 649: "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."
However, I agree that the behavior described here is unfortunate. Here's a possible way to fix it: In module and class scopes, assign each annotated assignment an identifier that is unique within that scope (e.g., a small integer starting at 0). Maintain an internal set (hidden to the user) that represents the assignments that have been executed. Add to the set whenever an annotated assignment is executed. Inside the generated
__annotate__, emit key/value pairs only for assignments that are in the executed set.Something like this:
I believe we could optimize away the
__executed_setcheck for assignments at the top level, which are executed unconditionally.CaselIT commentedon Mar 6, 2025
From the pep
I don't think the assumptions are correct, since it includes
if TYPE_CHECKINGthat I would argue is quite common.It's also wrong that type checker don't support such features. They certainly do then using
if TYPE_CHECKINGor static conditional such asif False.A quick glance at https://github.com/search?q=%22++++if+TYPE_CHECKING%22&type=code&p=1 also seems to indicate that such pattern is in use in at least in class definitions
CaselIT commentedon Mar 6, 2025
I think this will break all tools that use annotations to do runtime operations, for example dataclasses.
This does not work in 3.14a5
raises
Also it breaks other popular libreries. For example pydantic<2 does not even import in python 3.14:
This error if due to the
if TYPE_CHECKINGinside a class: https://github.com/pydantic/pydantic/blob/0cbba0f9774c470e8d8a2a62abb921b8cea4027d/pydantic/main.py#L316-L332I can't install pydantic>=2 since it has compile errors, but I would expect it to behave like dataclasses.
zzzeek commentedon Mar 6, 2025
@JelleZijlstra if we could have a simple flag on get_annotations() that simply says, "honor_conditionals=True" or something like that, this will allow libraries like pydantic / SQLAlchemy and others to not have to re-think our entire development model.
6 remaining items
CaselIT commentedon Mar 6, 2025
Indeed forgot about this case.
I would argue that it's worse though, since it this case no exception is raised and the print prints 2.
zzzeek commentedon Mar 6, 2025
on the SQLAlchemy side I've proposed two workarounds, one for end-user TYPE_CHECKING recipes we will provide an "ignore" sentinel like this, the existing use case looks like:
the user wants to apply
relationship()outside the class body but still have a type annotation for the attribute. We want SQLAlchemy to not attempt to runtime-evaluateMapped[list[Stuff]]since that will raise without the relationship being immediately present. A user with this use case will be able to force this annotation to be skipped at runtime as follows:so while SQLAlchemy declarative will get the
Mapped[list[Stuff]]annotation, it will also get theignore_mapped_attr()sentinel value so it will know to skip that. We also need to useget_annotations()withFormat.FORWARDREFsince we mostly prefer that ORM mapped annotations act likefrom __future__ import annotationsmode, that is, we need string names of related classes not the class itself. So it's nice that that's there at least.For our test suite, we have conditional suites that are just exercising many different syntaxes of annotations to test that SQLAlchemy can interpret them:
for these we have to write out the full "A" class each time for each conditional, which makes the test more verbose. This is a handful of test modules where we are trying to exercise parsing of many types of annotation and how they come in for different python versions, so hoping this is manageable. mostly I'm concerned with cognitively being able to catch suites like this which can silently fail to exercise the tests for the previous conditionals that are overwritten. seeing conditionals in source code and having to keep in one's mind that "part of this code will be not subject to the conditionals as though they weren't written, and this other part will be" is just unintuitive and strange.
CaselIT commentedon Mar 6, 2025
Just for fun I managed to make the interpreter run arbitrary code from
if Falseblocks.in file
other.pythen in another file
executing this prints
hi thereJelleZijlstra commentedon Mar 6, 2025
I am going to try to implement the fix proposed in my previous comment. Let's keep the discussion focused on how to fix this; we don't need more examples.
JelleZijlstra commentedon Mar 8, 2025
I proposed a fix: #130935. If you're interested in this issue, please try it out and let me know if you encounter any problems.
zzzeek commentedon Mar 9, 2025
ill see if i can try it out
zzzeek commentedon Mar 10, 2025
OK that seems to fix the conditional issue. Something else changed in 3.14 since a5 with Unions but I came up with a workaround for that.
gh-130881: Handle conditionally defined annotations (#130935)
pythongh-130881: Handle conditionally defined annotations (python#130935
hugovk commentedon Apr 7, 2025
Can this issue be closed now #130935 has been merged?
implement pep-649 workarounds, test suite passing for python 3.14
implement pep-649 workarounds, test suite passing for python 3.14