-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fix qualified name comparison of private attributes during namespace inspection #11803
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
Fix qualified name comparison of private attributes during namespace inspection #11803
Conversation
f51156b
to
06401d5
Compare
CodSpeed Performance ReportMerging #11803 will not alter performanceComparing Summary
|
please review |
06401d5
to
2cf3235
Compare
tests/test_private_attributes.py
Outdated
@@ -577,3 +577,24 @@ class Model(BaseModel): | |||
# Checks below are just to ensure that everything is the same as in `test_private_attr_set_name` | |||
# The main check is that model class definition above doesn't crash | |||
assert Model()._private_attr == 2 | |||
|
|||
|
|||
def test_models_contain_consistent_private_attributes_type(create_module): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test written in this way did not work:
def test_models_contain_consistent_private_attributes_type(create_module):
@create_module
def module():
import enum
from pydantic import BaseModel
class Table(enum.Enum):
TABLE_1 = enum.auto()
class FirstModel(BaseModel):
_table: enum.Enum = Table
class SecondModel(BaseModel):
_table: enum.Enum = Table
assert module.FirstModel._table == module.SecondModel._table
In other words, the error could not be reproduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test you added is unrelated to your change, which makes me believe your contribution is AI generated. If so, please state it explicitly in the PR description.
You can replace by the following test:
def test_private_attribute_not_skipped_during_ns_inspection() -> None:
# It is important for the enum name to start with the class name
# (it previously caused issues as we were comparing qualnames without
# taking this into account):
class Fullname(str, Enum):
pass
class Full(BaseModel):
_priv: object = Fullname
assert isinstance(Full._priv, ModelPrivateAttr)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn’t generated by AI — I just didn’t really know how to approach writing this test. I wanted to create a case as close as possible to the one in the issues. 🥲
Thanks for the example of how to write this test differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I finally understand where the problem in this test lies.
2cf3235
to
a472d2e
Compare
c695e68
to
9e77447
Compare
9e77447
to
d9bb773
Compare
Change Summary
Check if qualname startswith
namespace["__qualname__"]
with.
at the end.Related issue number
Fixes #11797
Checklist
Selected Reviewer: @DouweM