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
Fixed #35405 -- Used @cached_property in FieldCacheMixin. #18101
Conversation
a665863
to
424b695
Compare
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.
Thank you @adamchainz for this work! I agree with Simon that the patch looks good but I think this also needs a few tests covering the deprecation paths/messages and how both the old method and new cached property work OK.
973f80f
to
86d51aa
Compare
I have added some tests specific to the mixin, including for the deprecation path. |
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.
Branch looks great! I think is ready to checkin pending the adding of deprecation comments to the new test file so we can later cleanup accordingly. Thank you @adamchainz!
…eldCacheMixin. FieldCacheMixin is used by related fields to track their cached values. This work migrates get_cache_name() to be a cached property to optimize performance by reducing unnecessary function calls when working with related fields, given that its value remains constant. Co-authored-by: Simon Charette <charette.s@gmail.com> Co-authored-by: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Co-authored-by: Natalia <124304+nessita@users.noreply.github.com>
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.
Thank you Adam, Simon and Sarah!
Trac ticket number
ticket-35405
Branch description
Optimization as described on ticket.
Checklist
main
branch.