-
Notifications
You must be signed in to change notification settings - Fork 86
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
fix: ensure that __copy__
and __deepcopy__
are enabled.
#1695
Conversation
NB. I did think about introducing a `zeros_length` attribute, but I am concerned that we would need to take a great deal of care in handling this parameter for no good cause. Instead, we should make it such that the `length` parameter is the only externally visible one.
Codecov Report
Additional details and impacted files
|
This will ensure we *have* to implement this routine. Although we could use a dynamic deepcopy, it opens us up to more bugs (explicit vs implicit) and hides some of the copying logic.
This avoids handling deep copying in two places
We might want to also make proper use of the |
Thanks for the clean-ups! I checked each one and they are definitely improvements, though d02d927 is not actually related to this PR. Immutable objects that take children in their constructor arguments (without delayed initialization) can't have cycles, and we've made "layouts are trees" assumptions elsewhere, too. Nevertheless, I saw the code in the As you can see here: Python's Since your last commit was 2 hours ago, I can safely merge this. |
Yes, I was working off memory at the time of writing that comment. After checking the spec, I then went on to open #1697 when concerned about reference cycles (via mutable parameters), but subsequently recalled that we don't allow these, so it's a none-starter :) |
No description provided.