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

Use is_valid_field from 1.x for mypy plugin #8738

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

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

Copy link

codspeed-hq bot commented Feb 6, 2024

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?

@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

@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
Copy link
Member

@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
54 checks passed
@sydney-runkle sydney-runkle mentioned this pull request Feb 8, 2024
@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
Member

sydney-runkle commented Mar 18, 2024

@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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants