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

UP007 problems with pydantic and from __future__ import annotations #5434

Closed
Mr-Pepe opened this issue Jun 29, 2023 · 5 comments · Fixed by #5470
Closed

UP007 problems with pydantic and from __future__ import annotations #5434

Mr-Pepe opened this issue Jun 29, 2023 · 5 comments · Fixed by #5470
Assignees
Labels
configuration Related to settings and configuration

Comments

@Mr-Pepe
Copy link
Contributor

Mr-Pepe commented Jun 29, 2023

Consider the following file, written for Python3.9+.

# tmp.py
from __future__ import annotations

from typing import Union

from pydantic import BaseModel

x: Union[int, str] = 5


class A(BaseModel):
    y: Union[int, str] = 5

Running ruff check tmp.py --target-version py39 --select UP --fix with Ruff version 0.0.275 yields:

from __future__ import annotations

from typing import Union

from pydantic import BaseModel

x: int | str = 5


class A(BaseModel):
    y: int | str = 5

Running the script results in the following error:

Traceback (most recent call last):
  File "/home/felipe/Projects/voraus-vtest/tmp.py", line 10, in <module>
    class A(BaseModel):
  File "pydantic/main.py", line 178, in pydantic.main.ModelMetaclass.__new__
  File "pydantic/typing.py", line 400, in pydantic.typing.resolve_annotations
    
  File "/home/felipe/.pyenv/versions/3.9.13/lib/python3.9/typing.py", line 292, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "/home/felipe/.pyenv/versions/3.9.13/lib/python3.9/typing.py", line 554, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
TypeError: unsupported operand type(s) for |: 'type' and 'type'

The module-level x seems to work fine (because of the __future__ import?). Should Ruff maybe not change the type annotation if from __future__ import annotations is present? The type annotation might be evaluated in a different context where the import is not available.

@tmke8
Copy link
Contributor

tmke8 commented Jun 29, 2023

Yeah, the problem stems from the fact that pydantic evaluates the type annotations at runtime. I've had similar problems with hydra which also evaluates type annotations in order to do type validation at runtime.

Also note that from __future__ import annotations is generally problematic in pydantic: pydantic/pydantic#2678 which is why the Python steering council has not made it the default in Python 3.10 as originally planned, and instead has decided on the alternative mechanism proposed in PEP 649.

Ruff could check for these libraries and exempt them from UP007 but that would be a lot of special casing because I don't think you can in general tell whether a type annotation will be evaluated at runtime without knowing what the library does internally.

@eli-schwartz
Copy link
Contributor

While it's true ruff can special case imports of pydantic and just assume all annotations aren't safe to apply UP007 to, it's also true that UP007 is quite narrowly scoped and should arguably be disabled entirely in codebases that both support older versions of python, and use pydantic in some form.

A dedicated config setting for "i-use-runtime-annotations-please-dont-break-that = true" would allow UP007 to smartly detect when the minimum required python is updated and automatically re-enable itself.

@charliermarsh
Copy link
Member

charliermarsh commented Jun 30, 2023

Yeah, I generally recommend against using from __future__ import annotations with Pydantic for the above reasons (or disabling UP007).

We actually used to have a keep-runtime-typing flag, similar to Pyupgrade, but the way it was implemented meant that it was equivalent to disabling UP006 and UP007 entirely. I ended up deprecating it (#4427), since it felt odd to have special configuration that was exactly equivalent to just specifying the rules in an ignore, but I think I made a mistake (\cc @layday). We should've kept that flag, but fixed the semantics, such that it only disabled those rules for cases in which the syntax is not yet supported at runtime (i.e., < Python 3.9 or Python 3.10 depending on the rule). IMO we should restore it with those semantics.

@charliermarsh charliermarsh added the configuration Related to settings and configuration label Jun 30, 2023
@charliermarsh
Copy link
Member

I'm going to use this Issue as an opportunity to reintroduce that keep-runtime-typing setting.

@layday
Copy link

layday commented Jun 30, 2023

My suggestion would be to actually reverse the flag, so that people don't break things by accident.

@charliermarsh charliermarsh self-assigned this Jul 3, 2023
charliermarsh added a commit that referenced this issue Jul 3, 2023
## Summary

This PR reverts #4427. See the included documentation for a detailed
explanation.

Closes #5434.
zain-sohail added a commit to OpenCOMPES/sed that referenced this issue Jul 31, 2024
zain-sohail added a commit to OpenCOMPES/sed that referenced this issue Sep 16, 2024
rettigl pushed a commit to OpenCOMPES/sed that referenced this issue Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants