Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fix/deep copy recurse#29393
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
base:main
Are you sure you want to change the base?
Fix/deep copy recurse#29393
Conversation
ba83fb4 to60903f0CompareAlso, a point to stubtest for finding an unintended API change. |
Uh oh!
There was an error while loading.Please reload this page.
dd33ebc to8fedceaCompareWe provide deepcopy and copy methods as a shortcut for users to avoid having toimport the copy module and call the required function. Directly calling`__deepcopy__` side-steps all of the memoization machinery and may lead toincorrect results.
Seepython/cpython#126817 for upstream discussion.This works around the change by using (private) methods from the copy module tore-implement the path though copy/deepcopy that we would like to use but avoidthe special-casing for `super()` objects that is breaking us.We could vendor the current versions of `_keep_alive` (weakref work to managelifecycles) and `_reconstruct` (where the recursion happens) to superficiallyavoid using private functions from CPython. However, if these functions dochange significantly I worry that our copies would not inter-operate anyway.Closesmatplotlib#29157
8fedcea to717ea24CompareGiven that this is a) in regards to a pre-release version of python and b) awaiting a decision from upstream, I'm not going to hold 3.10.2 up on this. I consider this in the same bucket as#29816, as potentially deserving of a micro release upon merge, and certainly want it before 3.11.0, but not actually holding up micro releases. That said, retaining the |
power-cycled to get a new (synthetic) merge commit and re-start CI. We should probably review this and get it in sooner rather than later. There has been no motion upstream, but at this point I would rather get a 3.10.3 out with the fix assuming upstream does not change and contemplate reverting if they do rather than do nothing and then have to scramble later. |
PR summary
Seepython/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 managelifecycles) and
_reconstruct(where the recursion happens) to superficiallyavoid using private functions from CPython. However, if these functions do
change significantly I worry that our copies would not inter-operate anyway.
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