KEMBAR78
Use ``is_valid_field`` from 1.x for ``mypy`` plugin by DanielNoord · Pull Request #8738 · pydantic/pydantic · GitHub
Skip to content

Use is_valid_field from 1.x for mypy plugin #8738

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 7, 2024
Merged

Use is_valid_field from 1.x for mypy plugin #8738

merged 2 commits into from
Feb 7, 2024

Conversation

@DanielNoord
Copy link
Contributor

@DanielNoord DanielNoord commented Feb 6, 2024 •
edited by sydney-runkle
Loading

Change Summary

pydantic/pydantic/utils.py

Lines 696 to 699 in 3ddb509

def is_valid_field(name: str) -> bool:
if not name.startswith('_'):
return True
return ROOT_KEY == name

This brings the mypy plugin in the v1 namespace in line with how 1.x actually behaved.

Importing this from the pydantic namespace created warnings in our v1/v2 codebase.

Related issue number

No issue, couldn't find one and this seems like a simple enough change.

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 > seems unnecessary?
  • 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

skip change file check

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 6, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #8738 will not alter performance

Comparing DanielNoord:patch-1 (bd50c62) with main (3fda9d6)

Summary

✅ 10 untouched benchmarks

@DanielNoord
Copy link
Contributor Author

please review

I think the last failing test can be skipped by adding the correct label? It doesn't feel like this requires a changelog?

@sydney-runkle sydney-runkle added the relnotes-fix Used for bugfixes. label Feb 6, 2024
@dmontagu
Copy link
Contributor

dmontagu commented Feb 7, 2024

In general I think we want to avoid changing the 1.X module code as much as possible.

Does it not work to change:

from pydantic.utils import is_valid_field

to

from .utils import is_valid_field

?

@DanielNoord
Copy link
Contributor Author

DanielNoord commented Feb 7, 2024 •
edited
Loading

@dmontagu It should. Added a second commit to fix this, thanks!

Edit: For some reason CI fails. I can't really reproduce that environment locally easily. David would you mind rerunning CI and seeing if that fixes it? I don't see why PyPy3.10 should fail on this..

@sydney-runkle sydney-runkle changed the base branch from main to 1.10.X-fixes February 7, 2024 14:48
@sydney-runkle sydney-runkle changed the base branch from 1.10.X-fixes to main February 7, 2024 14:48
@sydney-runkle sydney-runkle changed the base branch from main to 1.10.X-fixes February 7, 2024 14:58
@sydney-runkle sydney-runkle changed the base branch from 1.10.X-fixes to main February 7, 2024 14:59
sydney-runkle
sydney-runkle approved these changes Feb 7, 2024
View reviewed changes
Copy link
Contributor

@sydney-runkle 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.

Wonderful, thanks!

@sydney-runkle sydney-runkle merged commit 3704ecc into pydantic:main Feb 7, 2024
@sydney-runkle sydney-runkle mentioned this pull request Feb 8, 2024
Merged
@DanielNoord
Copy link
Contributor Author

@sydney-runkle Any reason why this fix is not included in the latest releases? The backport to 1.10.x doesn't really resolve the issue, as the error is actually on the 2.x tags.

@sydney-runkle
Copy link
Contributor

sydney-runkle commented Mar 18, 2024 •
edited
Loading

@DanielNoord,

This should be included in 2.7! Thanks for the ping :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@sydney-runkle sydney-runkle sydney-runkle approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dmontagu dmontagu

@sydney-runkle sydney-runkle

Labels

ready for review relnotes-fix Used for bugfixes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DanielNoord @dmontagu @sydney-runkle