gh-100817: Speed up copy.deepcopy calls on slice objects - #100818
gh-100817: Speed up copy.deepcopy calls on slice objects#100818sobolevn wants to merge 1 commit into
Conversation
Sorry, something went wrong.
AlexWaygood
left a comment
There was a problem hiding this comment.
This PR results in a behaviour change for the following edge case.
On main:
>>> s = slice([1, 2], [3, 4], [5, 6])
>>> from copy import deepcopy
>>> t = deepcopy(s)
>>> t.start is s.start
FalseWith this PR:
>>> s = slice([1, 2], [3, 4], [5, 6])
>>> from copy import deepcopy
>>> t = deepcopy(s)
>>> t.start is s.start
True
Sorry, something went wrong.
|
Indeed, I haven't thought about this case. One of my main assumptions was that slice cannot contain nested structures. Because I've only seen int samples in real life. But, if lists are valid and used - this should not be merged. Because deepcopy does not work as promised: it is a shallow copy now. |
Sorry, something went wrong.
|
Yeah, it's a really obscure edge case, but I can find some examples in the wild: https://grep.app/search?q=%20slice%28%5B&case=true&words=true&filter[lang][0]=Python. Would you maybe like to add a test for this edge case? I very nearly merged the PR on the basis that all the tests passed, and it seemed like an impressive optimisation :) |
Sorry, something went wrong.
CC @AlexWaygood as the reviewer of #100818 Automerge-Triggered-By: GH:AlexWaygood
CC @AlexWaygood as the reviewer of python#100818 Automerge-Triggered-By: GH:AlexWaygood. (cherry picked from commit 729ab9b) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
CC @AlexWaygood as the reviewer of python#100818 Automerge-Triggered-By: GH:AlexWaygood. (cherry picked from commit 729ab9b) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
See the original issue for microbenchmarks.