NotificationsYou must be signed in to change notification settings
Fork2.2k
Star24.6k

Deprecate accessing`model_fields` and`model_computed_fields` on instances#11169

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

sydney-runkle merged 3 commits intomainfrom10930

Dec 30, 2024

Merged

Deprecate accessing`model_fields` and`model_computed_fields` on instances#11169

sydney-runkle merged 3 commits intomainfrom10930

Dec 30, 2024

Conversation

Copy link

Member

Viicos commentedDec 21, 2024•
edited
Loading

Change Summary

Fixes#10930.
Requires#11168, which defines the new deprecation class.

Related issue number

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

Deprecate accessingmodel_fields andmodel_computed_fields on ins…

efe378e

…tancesAlso reorder `BaseModel` members in API documentation.

github-actionsbot added the relnotes-fixUsed for bugfixes. label

Dec 21, 2024

Copy link

cloudflare-workers-and-pagesbot commentedDec 21, 2024•
edited
Loading

Deploying pydantic-docs with Cloudflare Pages

Latest commit:	`b871688`
Status:	✅ Deploy successful!
Preview URL:	https://7a2dc613.pydantic-docs.pages.dev
Branch Preview URL:	https://10930.pydantic-docs.pages.dev

View logs

Copy link

codspeed-hqbot commentedDec 21, 2024•
edited
Loading

CodSpeed Performance Report

Merging#11169 willnot alter performance

_{Comparing10930 (b871688) withmain (d823d8c)}

Summary

✅ 46 untouched benchmarks

Viicos force-pushed the10930 branch from99c071e to7339c67Compare

December 21, 2024 19:26

hyperlint-aibot reviewed

Dec 21, 2024

View reviewed changes

Copy link

Contributor

hyperlint-aibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The style guide flagged several spelling errors that seemed like false positives. We skipped posting inline suggestions for the following words:

[Mm]odel_fields

Viicos force-pushed the10930 branch from7339c67 to3639b49Compare

December 21, 2024 19:28

Copy link

Contributor

github-actionsbot commentedDec 21, 2024•
edited
Loading

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
pydantic
main.py
pydantic/_internal
_utils.py
Project Total

_{This report was generated bypython-coverage-comment-action}

Viicos commented

Dec 23, 2024

View reviewed changes

tests/test_computed_fields.py

		@@ -814,26 +814,3 @@ def my_field_serializer(self, value: Any, info: FieldSerializationInfo) -> Any:
		return f'{info.field_name} = {value}'

		assert MyModel().model_dump() == {'my_field': 'my_field = foo', 'other_field': 'other_field = 42'}


		def test_fields_on_instance_and_cls() -> None:

Copy link

MemberAuthor

ViicosDec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Tested intest_deprecated.py

Viicos added relnotes-changeUsed for changes to existing functionality which don't have a better categorization. and removed relnotes-fixUsed for bugfixes. labels

Dec 23, 2024

Viicos commented

Dec 23, 2024

View reviewed changes

tests/typechecking/base_model.py

		@@ -34,5 +34,6 @@ class Knight(BaseModel):

		assert_type(Knight.model_fields, dict[str, FieldInfo])
		assert_type(Knight.model_computed_fields, dict[str, ComputedFieldInfo])
		assert_type(k.model_fields, dict[str, FieldInfo])
		assert_type(k.model_computed_fields, dict[str, ComputedFieldInfo])
		# Mypy does not report the deprecated access (https://github.com/python/mypy/issues/18323):

Copy link

MemberAuthor

ViicosDec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

python/mypy#18333

sydney-runkle suggested changes

Dec 27, 2024

View reviewed changes

Copy link

Contributor

sydney-runkle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This looks good to me overall. Surprised that we had so many tests accessing these attributes on instances :(.

I've requested a few wording changes + maybe a chance to the already deprecated__fields__ as well.

I'd like to get input from@samuelcolvin on this PR before we merge, we can chat about this in our next sync 👍

pydantic/_internal/_model_construction.pyShow resolvedHide resolved

pydantic/_internal/_utils.py OutdatedShow resolvedHide resolved

pydantic-hookybot added awaiting author revisionawaiting changes from the PR author labels

Dec 27, 2024

pydantic-hookybot assignedViicos

Dec 27, 2024

Feedback

724fa34

Viicos force-pushed the10930 branch from9d8c97f to724fa34Compare

December 28, 2024 09:32

Use correct warning class

b871688

sydney-runkle approved these changes

Dec 30, 2024

View reviewed changes

Copy link

Contributor

sydney-runkle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thought about this a bit more. I definitely think this makes sense to deprecate.

We haven't lost any functionality here, so I'm alright with going ahead and merging. I don't think@samuelcolvin would object to a simple deprecation here.

Copy link

MemberAuthor

Viicos commentedDec 30, 2024

Yes to be clear, this was already deprecated by#10493, but this goes one step further by actually raising the deprecation warning and type checking error