Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Implement Path.__deepcopy__ avoiding infinite recursion#30198

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

jkseppan
Copy link
Member

@jkseppanjkseppan commentedJun 21, 2025•
edited
Loading

ImplementPath.__deepcopy__ avoiding infinite recursion

To deep copy an object without calling deepcopy on the object itself, create a new object of the correct class and iterate calling deepcopy on its__dict__.

Closes#29157 without relying on private CPython methods.

In a separate commit, fix the other issue withTransformNode.__copy__.

PR summary

PR checklist

@jkseppanjkseppan mentioned this pull requestJun 21, 2025
2 tasks
@jkseppanjkseppanforce-pushed thepath-deepcopy-via-metaclass branch 2 times, most recently fromfa7ed8b to68d25b3CompareJune 21, 2025 08:00
@jkseppanjkseppan requested a review fromtacaswellJune 21, 2025 08:56
@jkseppan
Copy link
MemberAuthor

jkseppan commentedJun 21, 2025•
edited
Loading

I tried the same strategy withTransformNode.__copy__ butcopy.copy looks atgetattr(cls, "__copy__", None) so we'd need to override attribute lookup on the class and not its instances, and then we can't have an instance-specific flag.

EDIT: this is actually easier to fix by just doing the shallow copy withoutcopy.copy

@jkseppan
Copy link
MemberAuthor

jkseppan commentedJun 23, 2025•
edited
Loading

I changed this to remove the metaclass and implement the__deepcopy__ method without calling it recursively. Also made sure that the tests check that thereadonly property is reset in the copy. (The branchpath-deepcopy-via-metaclass is now misnamed.)

if memo is None:
memo = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is not needed,memo is always a dict.

The default value formemo is also not needed. Thememo argument is always passed to__deepcopy__().

Copy link
MemberAuthor

@jkseppanjkseppanJun 24, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yes, when called viacopy.deepcopy, but this__deepcopy__ method is also aliased todeepcopy (just below the GitHub patch context) that is part of the public API of this class

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

we could change our deepcopy to callcopy.deepcopy internally (which might bet better?)

jkseppan reacted with thumbs up emoji
@jkseppanjkseppanforce-pushed thepath-deepcopy-via-metaclass branch 4 times, most recently from2d93b8e toa76ab5aCompareJune 25, 2025 02:59
jkseppanand others added3 commitsJune 25, 2025 07:06
To deep copy an object without calling deepcopy on the object itself,create a new object of the correct class and iterate calling deepcopy onits __dict__.Closesmatplotlib#29157 without relying on private CPython methods.Does not fix the other issue with TransformNode.__copy__.Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
without calling copy.copy
@jkseppanjkseppanforce-pushed thepath-deepcopy-via-metaclass branch fromdb1a305 to5c7c915CompareJune 25, 2025 04:07
Copy link
Member

@QuLogicQuLogic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I tried this out on Fedora Rawhide with Python 3.14, and it worked.

@tacaswelltacaswell added this to thev3.10.4 milestoneJun 26, 2025
@tacaswelltacaswell merged commitd05b43d intomatplotlib:mainJun 26, 2025
40 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestJun 26, 2025
@tacaswell
Copy link
Member

Thank you@jkseppan ! This is a better solution than my "reach up and touch private methods" version.

@jkseppanjkseppan deleted the path-deepcopy-via-metaclass branchJune 26, 2025 03:11
QuLogic added a commit that referenced this pull requestJun 26, 2025
…198-on-v3.10.xBackport PR#30198 on branch v3.10.x (Implement Path.__deepcopy__ avoiding infinite recursion)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka left review comments

@tacaswelltacaswelltacaswell approved these changes

@QuLogicQuLogicQuLogic approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.10.4
Development

Successfully merging this pull request may close these issues.

FUTURE BUG: reconsider how we deep-copy path objects
4 participants
@jkseppan@tacaswell@QuLogic@serhiy-storchaka
[8]ページ先頭
©2009-2025 Movatter.jp