GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||
GeezerVison is showing github.com — Fix/deep copy recurse by tacaswell · Pull Request #29393 · matplotlib/matplotlib Conversation
Member
PR summarySee python/cpython#126817 for upstream discussion. This works around the change by using (private) methods from the copy module to We could vendor the current versions of Closes #29157 Without these changes there are two classes of failures
The tests (mostly) pass [^] for me locally with a py314 build with this branch (and you have to be on 3.14 to hit the new code!) I think we should hold of merging this until closer to py314 being release (so some time over the summer) to see if upstream will change their mind about the change that is affecting us, but if we do need to merge this we should backport it has as we can as this is a show-stopping issue (infinite loop on render). The new code should work on all our supported versions of Python, but at least for now my view is we should version gate the fishy stuff (using private methods) to only the versions of Python where we have to. I have no idea if this is going to work as expected on pypy. PR checklist
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
tacaswell
force-pushed
the
fix/deep_copy_recurse
branch
2 times, most recently
from
ba83fb4 to
60903f0
Compare
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
highvelcty
reviewed
Uh oh!
There was an error while loading. Please reload this page.
tacaswell
force-pushed
the
fix/deep_copy_recurse
branch
from
dd33ebc to
8fedcea
Compare
1 of 2 tasks
tacaswell
force-pushed
the
fix/deep_copy_recurse
branch
from
8fedcea to
717ea24
Compare
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
ksunden
reviewed
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assert may not be necessary anymore
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsWe provide deepcopy and copy methods as a shortcut for users to avoid having to import the copy module and call the required function. Directly calling `__deepcopy__` side-steps all of the memoization machinery and may lead to incorrect results. See python/cpython#126817 for upstream discussion. This works around the change by using (private) methods from the copy module to re-implement the path though copy/deepcopy that we would like to use but avoid the special-casing for `super()` objects that is breaking us. We could vendor the current versions of `_keep_alive` (weakref work to manage lifecycles) and `_reconstruct` (where the recursion happens) to superficially avoid using private functions from CPython. However, if these functions do change significantly I worry that our copies would not inter-operate anyway. Closes matplotlib#29157
tacaswell
force-pushed
the
fix/deep_copy_recurse
branch
from
717ea24 to
e9c66d9
Compare
ksunden
approved these changes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Obviously some reservations in regards to using private methods from Python, but appears to be necessary. And would like to move forward with 3.14 support.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
1 task done
|
There was an error while loading. Please reload this page.
|
#30198 is an alternative fix without using private methods. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Closed in favor of #30198 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
FUTURE BUG: reconsider how we deep-copy path objects
There was an error while loading. Please reload this page.