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

Union of Annotated fields do not respect Field(strict=False) #9206

Open
1 task done
tropicoo opened this issue Apr 10, 2024 · 8 comments
Open
1 task done

Union of Annotated fields do not respect Field(strict=False) #9206

tropicoo opened this issue Apr 10, 2024 · 8 comments
Assignees
Labels
bug V2 Bug related to Pydantic V2 good first issue help wanted Pull Request welcome
Milestone

Comments

@tropicoo
Copy link

tropicoo commented Apr 10, 2024

Initial Checks

  • I confirm that I'm using Pydantic V2

Description

Cannot override strict mode for the annotated union of NewPath and Filepath fields when strict mode is set for all fields via ConfigDict.

Example Code

from pydantic import BaseModel, ConfigDict, Field, FilePath, NewPath
from typing_extensions import Annotated


class TestModel(BaseModel):
    results_filepath: NewPath | FilePath


class StrictTestModel(BaseModel):
    model_config = ConfigDict(strict=True)

    results_filepath: (
        Annotated[NewPath, Field(strict=False)]
        | Annotated[FilePath, Field(strict=False)]
    )


data = {'results_filepath': 'test.json'}

non_strict_model = TestModel(**data)
print(f'Non-strict model: {non_strict_model.model_dump()}\n')

strict_model = StrictTestModel(**data)                    # Exception raised
print(f'Strict model: {strict_model.model_dump()}')


"""
Output:

Non-strict model: {'results_filepath': WindowsPath('test.json')}

Traceback (most recent call last):
  File "***\pydantic_test.py", line 23, in <module>
    strict_model = StrictTestModel(**data)
                   ^^^^^^^^^^^^^^^^^^^^^^^
  File "***\venv_win3122\Lib\site-packages\pydantic\main.py", line 171, in __init__
    self.__pydantic_validator__.validate_python(data, self_instance=self)
pydantic_core._pydantic_core.ValidationError: 2 validation errors for StrictTestModel
results_filepath.function-after[validate_new(), lax-or-strict[lax=union[json-or-python[json=function-after[path_validator(), str],python=is-instance[Path]],function-after[path_validator(), str]],strict=json-or-python[json=function-after[path_validator(), str],python=is-instance[Path]]]]
  Input should be an instance of Path [type=is_instance_of, input_value='test.json', input_type=str]
    For further information visit https://errors.pydantic.dev/2.6/v/is_instance_of
results_filepath.function-after[validate_file(), lax-or-strict[lax=union[json-or-python[json=function-after[path_validator(), str],python=is-instance[Path]],function-after[path_validator(), str]],strict=json-or-python[json=function-after[path_validator(), str],python=is-instance[Path]]]]
  Input should be an instance of Path [type=is_instance_of, input_value='test.json', input_type=str]
    For further information visit https://errors.pydantic.dev/2.6/v/is_instance_of
"""

Python, Pydantic & OS Version

pydantic version: 2.6.4
        pydantic-core version: 2.16.3
          pydantic-core build: profile=release pgo=true
                 install path: ***\venv_win3122\Lib\site-packages\pydantic
               python version: 3.12.2 (tags/v3.12.2:6abddd9, Feb  6 2024, 21:26:36) [MSC v.1937 64 bit (AMD64)]
                     platform: Windows-10-10.0.19045-SP0
             related packages: fastapi-0.110.0 pydantic-settings-2.2.1 typing_extensions-4.10.0
                       commit: unknown
@tropicoo tropicoo added bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation labels Apr 10, 2024
@sydney-runkle
Copy link
Member

@tropicoo,

Hmph, thanks for reporting this. Definitely a bug - can likely be fixed in the schema generation logic in pydantic. PRs welcome with a fix!

@sydney-runkle sydney-runkle added good first issue help wanted Pull Request welcome and removed pending Awaiting a response / confirmation labels Apr 14, 2024
@sydney-runkle sydney-runkle added this to the v2.8.0 milestone Apr 14, 2024
@SharathHuddar
Copy link

@sydney-runkle I can pick this up if no one is working on this. Can you please assign this to me?

@SharathHuddar
Copy link

The issue comes from the output of pydantic.plugin._schema_validator.create_schema_validator. create_schema_validator internally calls SchemaValidator from pydantic-core. The logic to build the validator in pydantic-core needs to be updated here

@sydney-runkle @samuelcolvin thoughts?

@sydney-runkle
Copy link
Member

@SharathHuddar,

Sounds good, feel free to open a PR, I'd be happy to review!

@SharathHuddar
Copy link

@sydney-runkle I'm not able to figure out the right way to solve this. Please feel free to re-assign it to someone else

@dmontagu
Copy link
Contributor

It seems that the error happens because of the union of annotated. Specifically, this has the problem:

class StrictTestModel(BaseModel):
    model_config = ConfigDict(strict=True)

    results_filepath: (
        Annotated[NewPath, Field(strict=False)]
        | Annotated[FilePath, Field(strict=False)]
    )

but this doesn't:

class StrictTestModel(BaseModel):
    model_config = ConfigDict(strict=True)

    results_filepath: (
        Annotated[NewPath, Field(strict=False)]
    )

I think what's going on is that pydantic-core is not doing the right thing when determining the strictness to use within a union schema. There are a lot of ways this could be happening, I know @davidhewitt did some changes to union schema stuff a long time ago in the interest of trying to determine which case of the union was best-suited, and I wonder if the problem is it's trying to evaluate the inner schemas in strict mode under circumstances in which it should only try in lax mode. I think this is a significant step in the right direction of understanding what is going on BUT I wouldn't be surprised if it still takes a decent amount of work investigating inside pydantic-core to find the ultimate issue.

For what it's worth, @davidhewitt I think there's a chance you'd be able to figure this out quickly.

@mikeleppane
Copy link

mikeleppane commented Jun 5, 2024

Hey,

Indeed, the field's strict parameter is not respected when validating union type. If I'm not incorrect, it seems that UnionValidator only uses its "global" strict_mode (from ValidationState) instead of what's encoded into a field?

https://github.com/pydantic/pydantic-core/blob/34d789fc510810eee507347d5e0897d8fb9045bb/src/validators/union.rs#L123

https://github.com/pydantic/pydantic-core/blob/34d789fc510810eee507347d5e0897d8fb9045bb/src/validators/model_fields.rs#L123


Test examples:

Here are a couple of test examples with ModelValidator

This does not work:

def test_model_field_with_union_type_and_strict_mode_disabled():
    class MyModel:
        __slots__ = (
            "__dict__",
            "__pydantic_fields_set__",
            "__pydantic_extra__",
            "__pydantic_private__",
        )

    v = SchemaValidator(
        {
            "type": "model",
            "cls": MyModel,
            "schema": {
                "type": "union",
                "strict": True,
                "choices": [
                    {
                        "type": "model-fields",
                        "fields": {
                            "foo": {
                                "type": "model-field",
                                "strict": False,
                                "schema": {"type": "int"},
                            }
                        },
                    },
                    {
                        "type": "model-fields",
                        "fields": {
                            "bar": {
                                "type": "model-field",
                                "strict": False,
                                "schema": {"type": "int"},
                            }
                        },
                    },
                ],
            },
        }
    )
    m = v.validate_python({"foo": "123"})

Validator:

ModelValidator {
        revalidate: Never,
        validator: Union(
            UnionValidator {
                mode: Smart,
                choices: [
                    (
                        ModelFields(
                            ModelFieldsValidator {
                                fields: [
                                    Field {
                                        name: "foo",
                                        lookup_key: Simple {
                                            key: "foo",
                                            py_key: Py(
                                                0x00007f7d48d805d0,
                                            ),
                                            path: LookupPath(
                                                [
                                                    S(
                                                        "foo",
                                                        Py(
                                                            0x00007f7d48d82d90,
                                                        ),
                                                    ),
                                                ],
                                            ),
                                        },
                                        name_py: Py(
                                            0x00007f7d4bc194a0,
                                        ),
                                        validator: Int(
                                            IntValidator {
                                                strict: false,
                                            },
                                        ),
                                        frozen: false,
                                    },
                                ],
                                model_name: "Model",
                                extra_behavior: Ignore,
                                extras_validator: None,
                                strict: false,
                                from_attributes: false,
                                loc_by_alias: true,
                            },
                        ),
                        None,
                    ),
                    (
                        ModelFields(
                            ModelFieldsValidator {
                                fields: [
                                    Field {
                                        name: "bar",
                                        lookup_key: Simple {
                                            key: "bar",
                                            py_key: Py(
                                                0x00007f7d48d80630,
                                            ),
                                            path: LookupPath(
                                                [
                                                    S(
                                                        "bar",
                                                        Py(
                                                            0x00007f7d48d810e0,
                                                        ),
                                                    ),
                                                ],
                                            ),
                                        },
                                        name_py: Py(
                                            0x00007f7d4bc19440,
                                        ),
                                        validator: Int(
                                            IntValidator {
                                                strict: false,
                                            },
                                        ),
                                        frozen: false,
                                    },
                                ],
                                model_name: "Model",
                                extra_behavior: Ignore,
                                extras_validator: None,
                                strict: false,
                                from_attributes: false,
                                loc_by_alias: true,
                            },
                        ),
                        None,
                    ),
                ],
                custom_error: None,
                strict: true,
                name: "union[model-fields,model-fields]",
            },
        ),
        class: Py(
            0x00005561e20c1fd0,
        ),
        post_init: None,
        frozen: false,
        custom_init: false,
        root_model: false,
        undefined: Py(
            0x00007f7d4c5c13a0,
        ),
        name: "MyModel",
    }

This works:

def test_model_field():
    class MyModel:
        __slots__ = (
            "__dict__",
            "__pydantic_fields_set__",
            "__pydantic_extra__",
            "__pydantic_private__",
        )

    v = SchemaValidator(
        {
            "type": "model",
            "cls": MyModel,
            "schema": {
                "type": "union",
                "strict": True,
                "choices": [
                    {
                        "type": "model-fields",
                        "fields": {
                            "foo": {
                                "type": "model-field",
                                "strict": False,
                                "schema": {"type": "int"},
                            }
                        },
                    },
                ],
            },
        }
    )
    m = v.validate_python({"foo": "123"})

Validator:

ModelValidator {
        revalidate: Never,
        validator: ModelFields(
            ModelFieldsValidator {
                fields: [
                    Field {
                        name: "foo",
                        lookup_key: Simple {
                            key: "foo",
                            py_key: Py(
                                0x00007fd5b5b191d0,
                            ),
                            path: LookupPath(
                                [
                                    S(
                                        "foo",
                                        Py(
                                            0x00007fd5b72a1f20,
                                        ),
                                    ),
                                ],
                            ),
                        },
                        name_py: Py(
                            0x00007fd5b83154a0,
                        ),
                        validator: Int(
                            IntValidator {
                                strict: false,
                            },
                        ),
                        frozen: false,
                    },
                ],
                model_name: "Model",
                extra_behavior: Ignore,
                extras_validator: None,
                strict: false,
                from_attributes: false,
                loc_by_alias: true,
            },
        ),
        class: Py(
            0x00005593d66b1880,
        ),
        post_init: None,
        frozen: false,
        custom_init: false,
        root_model: false,
        undefined: Py(
            0x00007fd5b8cbd3a0,
        ),
        name: "MyModel",
    }

@sydney-runkle
Copy link
Member

@mikeleppane,

Great analysis here. Will definitely be helpful to have if we move forward with this as a desired change!

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 good first issue help wanted Pull Request welcome
Projects
None yet
Development

No branches or pull requests

5 participants