Handle a case when model_config is defined as a model property #9004

alexeyt101 · 2024-03-13T18:15:35Z

Change Summary

The PR aims to solve behavior when pydantic allows to define model_config as a model property with no default value. It leads to confusing behavior:

from pydantic import BaseModel


class MyModel(BaseModel):
    model_config: str


if __name__ == '__main__':
    m = MyModel(model_config='Test')
    print(m.model_config)
    """
    {}
    """

The MR offers to raise PydanticUserError in this case.

Related issue number

fix #8469

Checklist

The pull request title is a good summary of the changes - it will be used in the changelog
Unit tests for the changes exist
Tests pass on CI
Documentation reflects the changes where applicable
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @sydney-runkle

alexeyt101 · 2024-03-13T18:16:07Z

please review @sydney-runkle

codspeed-hq · 2024-03-13T18:19:28Z

CodSpeed Performance Report

Merging #9004 will not alter performance

_{Comparing alexeyt101:issue-8469 (2149ee1) with main (325ddf6)}

Summary

✅ 10 untouched benchmarks

sydney-runkle

Hi @alexeyt101,

Great work thus far! I've requested a few changes, mostly related to changing the name of the type of the error being raised.

Additionally, I wanted to bring up the question of what might happen if you use model_config as a field name in a pydantic dataclass? I haven't tested this out yet, but it'd be good to add a test for. I'm not sure whether or not we should error in that case...

Ping me when you've made the changes, I'd be happy to review again :). Thanks for your great work!

sydney-runkle · 2024-03-17T13:46:56Z

pydantic/errors.py

@@ -61,6 +61,7 @@
    'unevaluable-type-annotation',
    'dataclass-init-false-extra-allow',
    'clashing-init-and-init-var',
+    'model-config-cannot-be-a-property',


Let's use model-config-invalid-field-name instead

sydney-runkle · 2024-03-17T13:47:49Z

docs/errors/usage_errors.md

@@ -1089,4 +1089,18 @@ pydantic.errors.PydanticUserError: Dataclass field bar has init=False and init_v
 """
 ```

+## `model_config` is used as a model property {#model-config-cannot-be-a-property}


Below I suggested that we use model-config-invalid-field-name. Could you please change these docs to reflect that as well? I think we should use the term "model field" rather than property, just to keep things consistent 👍

sydney-runkle · 2024-03-17T13:48:37Z

pydantic/_internal/_config.py

+        raw_annotations = namespace.get('__annotations__', {})
+        if raw_annotations.get('model_config') and not config_dict_from_namespace:
+            raise PydanticUserError(
+                '"model_config" cannot be used as a model property. Use "model_config" for model configuration',


same suggestion here, let's use model field instead of model property

used model field

sydney-runkle · 2024-03-17T13:50:30Z

tests/test_config.py

+    """
+    Test: raises PydandicUserError when 'model_config' is used
+    as a property
+    """


Though I appreciate the extra documentation, I think if you just rename this test to something like test_model_config_as_model_field_raises, the test speaks for itself :).

I agree, thanks :)

alexeyt101 · 2024-03-17T17:11:53Z

Hi @sydney-runkle
I think we shouldn't raise the error if model_config is used as a model field in dataclass because it works clear and expected. I added a test that covers the case.

PS: I've closed the PR accidentally and hope it's not a problem :)

sydney-runkle · 2024-03-17T17:56:42Z

I think we shouldn't raise the error if model_config is used as a model field in dataclass

I'm fine with this for now. If it comes up in the future, we can readdress. I haven't seen any complaints thus far!

sydney-runkle

Requested a few more docs changes. Thanks!

sydney-runkle · 2024-03-17T17:57:20Z

docs/errors/usage_errors.md

+## `model_config` is used as a model field {#model-config-invalid-field-name}
+This error is raised when `model_config` is used as as property.


Suggested change

## `model_config` is used as a model field {#model-config-invalid-field-name}

This error is raised when `model_config` is used as as property.

## `model_config` is used as a model field {#model-config-invalid-field-name}

This error is raised when `model_config` is used as the name of a field.

sydney-runkle · 2024-03-17T17:58:25Z

pydantic/_internal/_config.py

+        raw_annotations = namespace.get('__annotations__', {})
+        if raw_annotations.get('model_config') and not config_dict_from_namespace:
+            raise PydanticUserError(
+                '"model_config" cannot be used as a model field. Use "model_config" for model configuration',


Suggested change

'"model_config" cannot be used as a model field. Use "model_config" for model configuration',

'`model_config` cannot be used as a model field name. Use `model_config` for model configuration.',

sydney-runkle · 2024-03-17T18:01:18Z

tests/test_config.py

+    model_field_title = 'model_field'
+
+    @pydantic_dataclass(config={'title': config_title})
+    class MyDataclass:
+        model_config: dict
+
+    m = MyDataclass(model_config={'title': model_field_title})
+
+    assert m.model_config['title'] == model_field_title


Suggested change

model_field_title = 'model_field'

@pydantic_dataclass(config={'title': config_title})

class MyDataclass:

model_config: dict

m = MyDataclass(model_config={'title': model_field_title})

assert m.model_config['title'] == model_field_title

field_title = 'from_field'

@pydantic_dataclass(config={'title': config_title})

class MyDataclass:

model_config: dict

m = MyDataclass(model_config={'title': field_title})

assert m.model_config['title'] == field_title

Thanks for you comments, I added all of it

sydney-runkle

Looks great, thanks for your awesome work on this!

sydney-runkle · 2024-03-17T18:53:00Z

Just as a side note, I'm not too worried about case 2 from the original issue, as it's discouraged to use that style of config specification in V2.

Handle a case when model_config is defined as a model property

3af130e

pydantic-hooky bot added the ready for review label Mar 13, 2024

pydantic-hooky bot assigned sydney-runkle Mar 13, 2024

sydney-runkle added the relnotes-feature label Mar 17, 2024

sydney-runkle requested changes Mar 17, 2024

View reviewed changes

pydantic-hooky bot added awaiting author revision and removed ready for review labels Mar 17, 2024

pydantic-hooky bot assigned alexeyt101 and unassigned sydney-runkle Mar 17, 2024

alexeyt101 added 2 commits March 17, 2024 18:59

issue-8469: changes the code of added error and related docs

218b6e6

issue-8469: adds a test of model_config usage in dataclasses

f58744d

alexeyt101 closed this Mar 17, 2024

alexeyt101 reopened this Mar 17, 2024

alexeyt101 requested a review from sydney-runkle March 17, 2024 17:12

sydney-runkle requested changes Mar 17, 2024

View reviewed changes

issue-8469: fix docs and some naming

2149ee1

alexeyt101 requested a review from sydney-runkle March 17, 2024 18:22

sydney-runkle approved these changes Mar 17, 2024

View reviewed changes

sydney-runkle merged commit 1cfb22e into pydantic:main Mar 17, 2024
53 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle a case when model_config is defined as a model property #9004

Handle a case when model_config is defined as a model property #9004

alexeyt101 commented Mar 13, 2024 •

edited by pydantic-hooky bot

alexeyt101 commented Mar 13, 2024 •

edited

codspeed-hq bot commented Mar 13, 2024 •

edited

sydney-runkle left a comment

sydney-runkle Mar 17, 2024

alexeyt101 Mar 17, 2024

sydney-runkle Mar 17, 2024

alexeyt101 Mar 17, 2024

sydney-runkle Mar 17, 2024

alexeyt101 Mar 17, 2024

sydney-runkle Mar 17, 2024

alexeyt101 Mar 17, 2024

alexeyt101 commented Mar 17, 2024

sydney-runkle commented Mar 17, 2024

sydney-runkle left a comment

sydney-runkle Mar 17, 2024

sydney-runkle Mar 17, 2024

sydney-runkle Mar 17, 2024

alexeyt101 Mar 17, 2024

sydney-runkle left a comment

sydney-runkle commented Mar 17, 2024

		## `model_config` is used as a model field {#model-config-invalid-field-name}
		This error is raised when `model_config` is used as as property.

	'"model_config" cannot be used as a model field. Use "model_config" for model configuration',
	'`model_config` cannot be used as a model field name. Use `model_config` for model configuration.',

Handle a case when model_config is defined as a model property #9004

Handle a case when model_config is defined as a model property #9004

Conversation

alexeyt101 commented Mar 13, 2024 • edited by pydantic-hooky bot

Change Summary

Related issue number

Checklist

alexeyt101 commented Mar 13, 2024 • edited

codspeed-hq bot commented Mar 13, 2024 • edited

Merging #9004 will not alter performance

Summary

sydney-runkle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexeyt101 commented Mar 17, 2024

sydney-runkle commented Mar 17, 2024

sydney-runkle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sydney-runkle left a comment

Choose a reason for hiding this comment

sydney-runkle commented Mar 17, 2024

alexeyt101 commented Mar 13, 2024 •

edited by pydantic-hooky bot

alexeyt101 commented Mar 13, 2024 •

edited

codspeed-hq bot commented Mar 13, 2024 •

edited