Skip to content

Conversation

tlambert03
Copy link
Contributor

@tlambert03 tlambert03 commented Jul 13, 2024

from pydantic import BaseModel, computed_field

class Thing(BaseModel):
    x: int

    @computed_field
    def y(self) -> bool: ...

    @y.setter
    def y(self, value: bool) -> None: ...

# this statement is True on `main` *only* if a setter is not used, otherwise it returns False
# this PR fixes the pointer when a setter is used.
print(Thing.model_computed_fields["y"].wrapped_property is Thing.y)

Change Summary

Fixes the pointer in ComputedFieldInfo.wrapped_property when a property setter is used

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

Selected Reviewer: @sydney-runkle

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Jul 13, 2024
Copy link

codspeed-hq bot commented Jul 13, 2024

CodSpeed Performance Report

Merging #9892 will not alter performance

Comparing tlambert03:fix-wrapped-prop (61fd877) with main (d654a07)

Summary

✅ 13 untouched benchmarks

@tlambert03 tlambert03 changed the title Fixe ComputedFieldInfo.wrapped_property pointer when a property setter is assigned Fix ComputedFieldInfo.wrapped_property pointer when a property setter is assigned Jul 13, 2024
@tlambert03
Copy link
Contributor Author

please review

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.

Great, thank you for fixing this @tlambert03!!

@sydney-runkle
Copy link
Contributor

Waiting to merge until we get a second pair of eyes from @dmontagu, he's more familiar with the decorator logic than I am :).

@dmontagu
Copy link
Contributor

I won't pretend to understand fully what's going on here, but this change seems reasonable and I can understand in principle why an update to the decorator metadata is necessary. Also the diff here leaves me feeling pretty confident that if this introduces any bugs, and that does seem unlikely, but if it does, they should be easy to address anyway. 👍

@dmontagu dmontagu merged commit 7cfaeba into pydantic:main Jul 18, 2024
@tlambert03 tlambert03 deleted the fix-wrapped-prop branch July 18, 2024 19:47
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.

3 participants