Skip to content

Do not error on fields overridden by methods in the mypy plugin#12290

Merged
Viicos merged 4 commits intomainfrom
mypy-plugin-frozen-overridden
Sep 30, 2025
Merged

Do not error on fields overridden by methods in the mypy plugin#12290
Viicos merged 4 commits intomainfrom
mypy-plugin-frozen-overridden

Conversation

@Viicos
Copy link
Copy Markdown
Member

@Viicos Viicos commented Sep 29, 2025

Change Summary

Also update mypy to 1.18 and update tests.

Fixes #12278.

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

@Viicos Viicos requested a review from davidhewitt September 29, 2025 11:59
@github-actions github-actions Bot added the relnotes-fix Used for bugfixes. label Sep 29, 2025
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Sep 29, 2025

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1b4dde9
Status: ✅  Deploy successful!
Preview URL: https://58a39027.pydantic-docs.pages.dev
Branch Preview URL: https://mypy-plugin-frozen-overridde.pydantic-docs.pages.dev

View logs

Comment thread tests/mypy/modules/frozen_field.py Outdated
parent_attr: str = Field(exclude=True)


# We don't wan't to froze `parent_attr` in the plugin:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not? also typo should be "freeze"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See the fixed issue, it results in unexpected errors.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see, maybe this is clearer:

Suggested change
# We don't wan't to froze `parent_attr` in the plugin:
# `parent_attr` is writable, mypy should error when overriding with a read-only property

Comment thread tests/mypy/modules/frozen_field.py Outdated


# We don't wan't to froze `parent_attr` in the plugin:
class Chield(Parent):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
class Chield(Parent):
class Child(Parent):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't look like this suggestion was applied?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hum I don't see it on the diff?

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Sep 29, 2025

CodSpeed Performance Report

Merging #12290 will improve performances by 5.19%

Comparing mypy-plugin-frozen-overridden (1b4dde9) with main (58befdc)

Summary

⚡ 1 improvement
✅ 45 untouched

Benchmarks breakdown

Benchmark BASE HEAD Change
test_north_star_json_loads 19.6 ms 18.6 ms +5.19%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 29, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic
  mypy.py 957
Project Total  

This report was generated by python-coverage-comment-action

@Viicos Viicos force-pushed the mypy-plugin-frozen-overridden branch from 5217563 to feb23f9 Compare September 29, 2025 21:06
@Viicos Viicos force-pushed the mypy-plugin-frozen-overridden branch from feb23f9 to 5a3337d Compare September 29, 2025 21:08
Copy link
Copy Markdown
Collaborator

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

I see, I understand now

Comment thread tests/mypy/modules/frozen_field.py Outdated
parent_attr: str = Field(exclude=True)


# We don't wan't to froze `parent_attr` in the plugin:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see, maybe this is clearer:

Suggested change
# We don't wan't to froze `parent_attr` in the plugin:
# `parent_attr` is writable, mypy should error when overriding with a read-only property

Comment thread tests/mypy/modules/frozen_field.py Outdated


# We don't wan't to froze `parent_attr` in the plugin:
class Chield(Parent):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't look like this suggestion was applied?

Comment thread tests/mypy/modules/frozen_field.py Outdated
Comment thread tests/mypy/outputs/mypy-plugin_ini/frozen_field.py Outdated
@Viicos Viicos enabled auto-merge (squash) September 30, 2025 12:43
@Viicos Viicos merged commit 9b438b4 into main Sep 30, 2025
61 checks passed
@Viicos Viicos deleted the mypy-plugin-frozen-overridden branch September 30, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

relnotes-fix Used for bugfixes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The pydantic mypy plugin ran into unexpected behavior [pydantic-unexpected]

2 participants