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

Using a default Field will crash when the type annotation references a class of the same name as the field #9093

Open
1 task done
lmmx opened this issue Mar 24, 2024 · 4 comments
Assignees
Labels
bug V2 Bug related to Pydantic V2

Comments

@lmmx
Copy link
Contributor

lmmx commented Mar 24, 2024

Initial Checks

  • I confirm that I'm using Pydantic V2

Description

Traceback (most recent call last):
  File "/home/louis/lab/tubeulator/pascalify/Journey.py", line 10, in <module>
    class X(BaseModel):
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py", line 178, in __new__
    set_model_fields(cls, bases, config_wrapper, types_namespace)
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py", line 452, in set_model_fields
    fields, class_vars = collect_model_fields(cls, bases, config_wrapper, types_namespace, typevars_map=typevars_map)
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/site-packages/pydantic/_internal/_fields.py", line 122, in collect_model_fields
    type_hints = get_cls_type_hints_lenient(cls, types_namespace)
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/site-packages/pydantic/_internal/_typing_extra.py", line 212, in get_cls_type_hints_lenient
    hints[name] = eval_type_lenient(value, globalns, localns)
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/site-packages/pydantic/_internal/_typing_extra.py", line 224, in eval_type_lenient
    return eval_type_backport(value, globalns, localns)
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/site-packages/pydantic/_internal/_typing_extra.py", line 240, in eval_type_backport
    return typing._eval_type(  # type: ignore
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/typing.py", line 327, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/typing.py", line 693, in _evaluate
    type_ = _type_check(
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/typing.py", line 176, in _type_check
    raise TypeError(f"{msg} Got {arg!r:.100}.")
TypeError: Forward references must evaluate to types. Got FieldInfo(annotation=NoneType, required=False).

This is a very odd and specific bug, I've tried changing aspects of it and have whittled it down to the reproducible essence, as in this issue's title.

  • It only occurs when you use Field, even when equivalent to not using Field (i.e. = Field(None) rather than = None)
  • It only occurs when the field name matches the [forward referenced] field type name
  • It is not affected by stringifying the type

This isn't some contrived example, it came up when trying to codegen for the TfL API.

I think I can work around this, it's a shame that I can't use aliases in this way though.

Example Code

from __future__ import annotations

from pydantic import BaseModel, Field


class Aaa(BaseModel):
    b: str = None


class X(BaseModel):
    Aaa: Aaa = Field(None)  # This crashes
    # Aaa: "Aaa" = Field(None)  # Likewise
    # Aaa: Aaa = None  # This is fine

Python, Pydantic & OS Version

pydantic version: 2.6.4
        pydantic-core version: 2.16.3
          pydantic-core build: profile=release pgo=true
                 install path: /home/louis/miniconda3/envs/tubeulator/lib/python3.10/site-packages/pydantic
               python version: 3.10.14 (main, Mar 21 2024, 16:24:04) [GCC 11.2.0]
                     platform: Linux-5.15.0-91-generic-x86_64-with-glibc2.35
             related packages: typing_extensions-4.10.0
                       commit: unknown
@lmmx lmmx added bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation labels Mar 24, 2024
@sydney-runkle
Copy link
Member

@lmmx,

Thanks for the detailed bug report. I know that @Viicos worked on a PR recently to disallow / warn for cases similar to yours above. @Viicos, any idea why that wouldn't take effect for this case (when running on main, where your change is active).

@sydney-runkle sydney-runkle removed the pending Awaiting a response / confirmation label Mar 25, 2024
@sydney-runkle sydney-runkle self-assigned this Mar 25, 2024
@Viicos
Copy link
Contributor

Viicos commented Mar 25, 2024

Aaa: Aaa = Field(None) should have been caught by #8243, however I don't think it works with the __future__ import. I'll try to take a look at it and see if it can be handled as well.

Aaa: Aaa = None seems to be the same as #8243 (comment), and can be very misleading as the Aaa annotation will take the value of None.

@lmmx My suggestion is to follow the alternatives mentioned here. I know this isn't perfect but Pydantic is limited by the annotation evaluation of Python (assignments are evaluated before annotations, meaning when Python tries to evaluate Aaa, it uses the assigned value in the class as it is in the same class scope). I'll also note that type checkers won't support the annotation as well (see this screenshot).

To catch this kind of issues, I wrote a Flake8 plugin (rule can be found here). This detection could be implemented in Pydantic but it would require parsing the AST which isn't an ideal solution considering it would slow down model creation.

@lmmx
Copy link
Contributor Author

lmmx commented Mar 25, 2024

Thanks for the prompt triage @sydney-runkle and explanation @Viicos ! Best team 💪

Due to the specific [imposed] requirements I have in this project:

  1. The code is codegen'd, some keys in the JSON it parses are reserved words in Python so would be illegal [in the AST] to leave as model field names, hence must be aliased from camelCase to PascalCase
  2. There are cyclic inter-model references (so forward refs must be stringified with the future annotations import)

mean that I can't use the alternatives but good to know!

I've opted to go with an AliasGenerator for now which seems to do the trick for my needs, still wanted to report it back to base :-) Plugin looks great too, I use flake8 all the time (even though my pre-commit is in ruff now so cool to see the plans for that!)

I went back and checked your warning that a stringified annotation - one example is the Identifier model I generated here which includes such a field:

class Identifier(BaseModel):
    Crowding: Crowding = None

Indeed you were right 😮‍💨

>>> from tubeulator.generated.Journey import Identifier
>>> Identifier.model_validate({})
Identifier(Id=None, Name=None, Uri=None, FullName=None, Type=None, Crowding=None, RouteType=None, Status=None)
>>> Identifier.model_validate({"crowding": {"passengerFlows": [], "trainLoadings": []}})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/louis/miniconda3/envs/tubeulator/lib/python3.10/site-packages/pydantic/main.py", line 509, in model_validate
    return cls.__pydantic_validator__.validate_python(
pydantic_core._pydantic_core.ValidationError: 1 validation error for Identifier
crowding
  Input should be None [type=none_required, input_value={}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.6/v/none_required

Thanks for the specific warning there, I thought I was good to go ahead ! I'll have a think about the other workarounds

Can confirm that the simple approach (giving the model type another name after declaring it, and then refer to it by that name in the field annotation) works for me! 🎉

>>> from tubeulator.generated.Journey import Identifier
>>> Identifier.model_validate({"crowding": {"passengerFlows": [], "trainLoadings": []}})
Identifier(Id=None, Name=None, Uri=None, FullName=None, Type=None, Crowding=Crowding(PassengerFlows=[], TrainLoadings=[]), RouteType=None, Status=None)

@Viicos
Copy link
Contributor

Viicos commented Mar 25, 2024

I use flake8 all the time (even though my pre-commit is in ruff now so cool to see the plans for that!)

My plan is to implement these rules at Ruff at some point (but still need to a bit more familiar with Rust first). But yeah aliases seems like a good option

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V2 Bug related to Pydantic V2
Projects
None yet
Development

No branches or pull requests

3 participants