Skip to content
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

bpo-29506: Fixes documentation for usage of deepcopy in copy module #151

Merged
merged 3 commits into from Apr 9, 2017

Conversation

Projects
None yet
7 participants
@CuriousLearner
Copy link
Contributor

commented Feb 18, 2017

Fixes issue: http://bugs.python.org/issue29506

Changes Steven's proposal to use word state instead of data as per suggestion from @ncoghlan

@the-knights-who-say-ni

This comment has been minimized.

Copy link

commented Feb 18, 2017

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow these steps to rectify the issue:

  1. Sign the PSF contributor agreement
  2. Wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
  3. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

@CuriousLearner

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2017

CLA is signed and now visible in my account.

cc @ncoghlan

@@ -47,8 +47,8 @@ copy operations:
* Recursive objects (compound objects that, directly or indirectly, contain a
reference to themselves) may cause a recursive loop.

* Because deep copy copies *everything* it may copy too much, e.g.,
even administrative data structures that should be shared even between copies.
* Because deep copy copies everything it may copy too much, such as state

This comment has been minimized.

Copy link
@marco-buttu

marco-buttu Mar 27, 2017

Contributor

Thanks @CuriousLearner. IMO the @stevendaprano wording, that refers to "data which is intended to be shared" ("Because deep copy copies everything, it may copy too much, such as data which is intended to be shared between copies."), is more clear.

This comment has been minimized.

Copy link
@CuriousLearner

CuriousLearner Mar 27, 2017

Author Contributor

@marco-buttu Sure, I will fix that.

@CuriousLearner

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2017

@marco-buttu I've done the changes, please have a look!

Thanks!

@marco-buttu

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2017

Thanks @CuriousLearner, it LGTM. @bitdancer proposed a shorten variant in pbo-29506:

Because deep copy copies everything, it may copy data that is intended to be shared between copies.

This variant also LGTM. Let's see what @bitdancer says.

@ncoghlan ncoghlan self-assigned this Apr 9, 2017

@ncoghlan ncoghlan merged commit 19e0494 into python:master Apr 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ncoghlan

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2017

3.6: #1061
3.5: #1062
2.7: #1063

@ncoghlan

This comment has been minimized.

Copy link
Contributor

commented May 28, 2017

Added the sprint label, as this PR was submitted at the PyCon Pune 2017 core development sprint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.