Skip to content

Conversation

karta9821
Copy link
Contributor

@karta9821 karta9821 commented Apr 28, 2025

Change Summary

Check if qualname startswith namespace["__qualname__"] with . at the end.

Related issue number

Fixes #11797

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

Selected Reviewer: @DouweM

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Apr 28, 2025
@karta9821 karta9821 force-pushed the fix/make-private-attribute-type-consistent branch from f51156b to 06401d5 Compare April 28, 2025 13:23
Copy link
Contributor

Coverage report

This PR does not seem to contain any modification to coverable code.

Copy link

codspeed-hq bot commented Apr 28, 2025

CodSpeed Performance Report

Merging #11803 will not alter performance

Comparing karta9821:fix/make-private-attribute-type-consistent (51d90bf) with main (455b436)

Summary

✅ 46 untouched benchmarks

@karta9821
Copy link
Contributor Author

please review

@karta9821 karta9821 force-pushed the fix/make-private-attribute-type-consistent branch from 06401d5 to 2cf3235 Compare April 28, 2025 13:49
@@ -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):
Copy link
Contributor Author

@karta9821 karta9821 Apr 28, 2025

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.

Copy link
Member

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)

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@karta9821 karta9821 force-pushed the fix/make-private-attribute-type-consistent branch from 2cf3235 to a472d2e Compare April 28, 2025 14:07
@Viicos Viicos changed the title Fix: Make private attribute type consistent across models Fix qualified name comparison of private attributes during namespace inspection Apr 28, 2025
@karta9821 karta9821 force-pushed the fix/make-private-attribute-type-consistent branch 3 times, most recently from c695e68 to 9e77447 Compare April 28, 2025 16:02
@karta9821 karta9821 force-pushed the fix/make-private-attribute-type-consistent branch from 9e77447 to d9bb773 Compare April 28, 2025 16:08
@DouweM DouweM assigned Viicos and unassigned DouweM Apr 28, 2025
@Viicos Viicos enabled auto-merge (squash) April 28, 2025 19:09
@Viicos Viicos merged commit 14a239c into pydantic:main Apr 28, 2025
59 checks passed
@karta9821 karta9821 deleted the fix/make-private-attribute-type-consistent branch May 16, 2025 11:52
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.

Inconsistent Private Attribute Type
3 participants