DOC: Update pandas.Series.copy docstring #20261

thismakessand · 2018-03-10T23:06:04Z

Closes #19505

Add extended summary
Add examples

Checklist for the pandas documentation sprint (ignore this if you are doing
an unrelated PR):

PR title is "DOC: update the docstring"
The validation script passes: scripts/validate_docstrings.py <your-function-or-method>
The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
The html version looks good: python doc/make.py --single <your-function-or-method>
It has been proofread on language by another sprint participant

Please include the output of the validation script below between the "```" ticks:


################################################################################
######################## Docstring (pandas.Series.copy) ########################
################################################################################

Make a copy of this objects indices and data.

Deep copy (default) will create a new object with a copy of the objects
data and indices. Modifications to the data or indices of the copy
will not be reflected in the original object.

Shallow copy (``deep=False``) will create a new object without copying
the data or indices. Any changes to the index or data of the original
will be reflected in the shallow copy (and vice versa).

Parameters
----------
deep : boolean or string, default True
    Make a deep copy, including a copy of the data and the indices.
    With ``deep=False`` neither the indices or the data are copied.

    Note that when ``deep=True`` data is copied, actual python objects
    will not be copied recursively, only the reference to the object.
    This is in contrast to ``copy.deepcopy`` in the Standard Library,
    which recursively copies object data.

Returns
-------
copy : type of caller

Examples
--------
>>> s = pd.Series([1,2], dtype="int32", index=["a", "b"])
>>> s
a    1
b    2
dtype: int32
>>> s_copy = s.copy()
>>> s_copy
a    1
b    2
dtype: int32

Shallow copy versus default (deep) copy:

In a shallow copy the index and data is a reference to the original
objects'.

>>> s = pd.Series([1,2], dtype="int32", index=["a", "b"])
>>> deep_copy = s.copy()
>>> shallow_copy = s.copy(deep=False)
>>> id(s) == id(shallow_copy)
False
>>> id(s.index) == id(shallow_copy.index)
True
>>> id(s.values) == id(shallow_copy.values)
True
>>> id(s) == id(deep_copy)
False
>>> id(s.index) == id(deep_copy.index)
False
>>> id(s.values) == id(deep_copy.values)
False
>>> s[0] = 3
>>> s
a    3
b    2
dtype: int32
>>> shallow_copy
a    3
b    2
dtype: int32
>>> deep_copy
a    1
b    2
dtype: int32
>>> shallow_copy[0] = 4
>>> s
a    4
b    2
dtype: int32
>>> shallow_copy
a    4
b    2
dtype: int32
>>> deep_copy
a    1
b    2
dtype: int32

When copying object containing python objects, deep copy will
copy the indices and data but will not do so recursively.

>>> class Point(object):
...     def __init__(self, x, y):
...         self.x = x
...         self.y = y
...     def __repr__(self):
...         return "Point(%d,%d)" % (self.x, self.y)
...
>>> p1 = Point(0, 1)
>>> s = pd.Series([p1], dtype="object")
>>> s
0    Point(0,1)
dtype: object
>>> s_copy = s.copy()
>>> s_copy
0    Point(0,1)
dtype: object
>>> id(s[0]) == id(s_copy[0])
True
>>> p1.x = 1
>>> s
0    Point(1,1)
dtype: object
>>> s_copy
0    Point(1,1)
dtype: object
>>>

For deep-copying python objects, the following can be used:

>>> import copy
>>> deep_deep_copy = pd.Series(copy.deepcopy(s.values),
...                            index=copy.deepcopy(s.index))

################################################################################
################################## Validation ##################################
################################################################################

Errors found:
        See Also section not found

If the validation script still gives errors, but you think there is a good reason
to deviate in this case (and there are certainly such cases), please state this
explicitly.

Not sure if anything should be added to the See Also section

- Add extended summary - Add examples

WillAyd · 2018-03-11T01:10:53Z

pandas/core/generic.py

+        b    2
+        dtype: int32
+
+        When copying object containing python objects, deep copy will


"an object"

WillAyd · 2018-03-11T01:12:04Z

pandas/core/generic.py

@@ -4506,7 +4506,15 @@ def astype(self, dtype, copy=True, errors='raise', **kwargs):

    def copy(self, deep=True):
        """
-        Make a copy of this objects data.
+        Make a copy of this objects indices and data.


For the sake of explicitness can you change "this objects" to "the calling object's" or "the caller's"

WillAyd · 2018-03-11T01:14:36Z

pandas/core/generic.py

-        Make a copy of this objects data.
+        Make a copy of this objects indices and data.
+
+        Deep copy (default) will create a new object with a copy of the objects


Given these start to dive into the parameters and implementations I think they are better served in a dedicated Notes section

WillAyd · 2018-03-11T01:16:36Z

pandas/core/generic.py

+        data and indices. Modifications to the data or indices of the copy
+        will not be reflected in the original object.
+
+        Shallow copy (``deep=False``) will create a new object without copying


Single back ticks

WillAyd · 2018-03-11T01:16:44Z

pandas/core/generic.py

-        Make a copy of this objects data.
+        Make a copy of this objects indices and data.
+
+        Deep copy (default) will create a new object with a copy of the objects


WillAyd · 2018-03-11T01:19:03Z

pandas/core/generic.py

+        b    2
+        dtype: int32
+
+        Shallow copy versus default (deep) copy:


Since this is a "section header" can you get it to render in bold (i.e. between asterisks)?

WillAyd · 2018-03-11T01:19:47Z

pandas/core/generic.py

+
+        Shallow copy versus default (deep) copy:
+
+        In a shallow copy the index and data is a reference to the original


Maybe just "the index and data are shared with the original object"

WillAyd · 2018-03-11T01:21:18Z

pandas/core/generic.py

+
+        Examples
+        --------
+        >>> s = pd.Series([1,2], dtype="int32", index=["a", "b"])


Is the dtype argument provided because you want it to be clear that type is copied over and not automatically upcast to int64? I think that's what you are going for but it wasn't immediately obvious to me, so I'd try and make it more explicit if that is your intention

WillAyd · 2018-03-11T01:24:43Z

pandas/core/generic.py

+        >>> s = pd.Series([1,2], dtype="int32", index=["a", "b"])
+        >>> deep_copy = s.copy()
+        >>> shallow_copy = s.copy(deep=False)
+        >>> id(s) == id(shallow_copy)


These examples are certainly comprehensive but perhaps too verbose. Can you refactor to make the index / values comparisons fit on one line? Similar comment with the value assignment - any consolidation will make it more readable

WillAyd · 2018-03-11T01:26:59Z

pandas/core/generic.py

+        When copying object containing python objects, deep copy will
+        copy the indices and data but will not do so recursively.
+
+        >>> class Point(object):


Is it required to put a class in this docstring? Again for the sake of being concise wonder if you can just use lists to illustrate

Thanks! I made this change plus the other requested changes.

thismakessand · 2018-03-11T03:21:19Z

I removed the wording/examples about the index not being copied when deep=False because while it's not copied when .copy() is called, if the index is changed on the shallow copy it does not affect the index of the original (in contrast with the data values, which are also not copied, but changes to values do affect the original).

WillAyd · 2018-03-11T03:32:59Z

pandas/core/generic.py

+        When `deep=False`, a new object will be created without copying
+        the calling object's data (only a reference to the data is
+        copied). Any changes to the data of the original will be reflected
+        in the shallow copy (and vice versa).

        Parameters
        ----------
        deep : boolean or string, default True


`True` instead of just True

WillAyd · 2018-03-11T03:35:34Z

pandas/core/generic.py

+        When `deep=False`, a new object will be created without copying
+        the calling object's data (only a reference to the data is
+        copied). Any changes to the data of the original will be reflected
+        in the shallow copy (and vice versa).

        Parameters
        ----------
        deep : boolean or string, default True


str instead of string

WillAyd · 2018-03-11T03:36:20Z

pandas/core/generic.py

-            will not be copied recursively, only the reference to the object.
-            This is in contrast to ``copy.deepcopy`` in the Standard Library,
-            which recursively copies object data.
+            With `deep=False` neither the indices or the data are copied.

        Returns
        -------
        copy : type of caller


Move "type of caller" down as the description and say something like "Object type matches caller."

WillAyd · 2018-03-11T03:36:51Z

pandas/core/generic.py


-        >>> s = pd.Series([1,2], dtype="int32", index=["a", "b"])
+        >>> s = pd.Series([1,2], index=["a", "b"])


Space after first comma

WillAyd · 2018-03-11T03:37:11Z

pandas/core/generic.py


-        When copying object containing python objects, deep copy will
-        copy the indices and data but will not do so recursively.
+        When copying an object containing python objects, deep copy will


Capitalize Python and maybe say "a deep copy"

WillAyd · 2018-03-11T03:39:48Z

pandas/core/generic.py

        >>> deep_copy = s.copy()
        >>> shallow_copy = s.copy(deep=False)
        >>> id(s) == id(shallow_copy)
        False
-        >>> id(s.index) == id(shallow_copy.index)
-        True
        >>> id(s.values) == id(shallow_copy.values)


Just again wondering if these can be more concise. Perhaps choosing a different variable name will allow you to do id(s.values) == id(shlw.values) and id(s.index) == id(shlw.values)

WillAyd · 2018-03-11T03:41:19Z

pandas/core/generic.py

+        When `deep=True`, data is copied but actual python objects
+        will not be copied recursively, only the reference to the object.
+        This is in contrast to `copy.deepcopy` in the Standard Library,
+        which recursively copies object data.  (See examples below).


"...copies object data (see examples below)." - reduces some punctuation and saves space

WillAyd · 2018-03-11T03:41:46Z

pandas/core/generic.py

-            will not be copied recursively, only the reference to the object.
-            This is in contrast to ``copy.deepcopy`` in the Standard Library,
-            which recursively copies object data.
+            With `deep=False` neither the indices or the data are copied.


"neither...nor" instead of "or"

WillAyd · 2018-03-11T03:43:15Z

pandas/core/generic.py

+        When `deep=True` (default), a new object will be created with a
+        copy of the calling object's data and indices. Modifications to
+        the data or indices of the copy will not be reflected in the
+        original object (see notes below).


Unless you want to have a Notes section I would say "see Examples below" as a reference to the section actually showing that

WillAyd · 2018-03-11T03:46:15Z

pandas/core/generic.py

        >>> deep_copy
        a    1
        b    2
-        dtype: int32
+        dtype: int64
        >>> shallow_copy[0] = 4


Instead of having two different assignments I think you can have s[0] = 3 and then this line both before printing any of the objects. Gets you the same result and covers the same concepts in less lines

WillAyd · 2018-03-11T03:59:16Z

@thismakessand not sure I fully understand why you removed the mention of indices. Perhaps its related to the conversation in #19862? Referencing that I think it's preferable to use is for comparing the indices instead of using the equality operator. Perhaps that will help clarify how the copy is actually working behind the scenes.

Lot's of edits but they are mostly minor - thanks and keep up the great work!

jreback · 2018-03-11T13:30:18Z

pandas/core/generic.py

+        copy of the calling object's data and indices. Modifications to
+        the data or indices of the copy will not be reflected in the
+        original object (see notes below).
+


indices are immutable, mention this

jreback · 2018-03-11T13:32:06Z

pandas/core/generic.py

+        a    1
+        b    2
+        dtype: int64
+        >>> s_copy = s.copy()


blank line between cases (if they are separate / distinct)

jreback · 2018-03-11T13:33:01Z

pandas/core/generic.py

+        False
+        >>> id(s.values) == id(deep_copy.values)
+        False
+        >>> s[0] = 3


break this up with some text describing what you are showing

jreback · 2018-03-11T13:33:37Z

pandas/core/generic.py

+        copy the data, but will not do so recursively.
+
+        >>> s = pd.Series([[1, 2], [3, 4]])
+        >>> s_copy = s.copy()


show this with a deep copy

I wasn't sure how to address this comment - do you mean to show the behavior of
.copy(deep=True) versus the std python copy.deepcopy()? I was trying to show an example of how a (deep=True) copy isn't necessarily a deep copy in the standard python sense, but maybe I should remove this example entirely since it's already mentioned as a caveat in the Notes section?

jreback · 2018-03-11T13:33:46Z

pandas/core/generic.py

+        1     [3, 4]
+        dtype: object
+
+        For deep-copying python objects, the following can be used:


this last example is not necessary

thismakessand · 2018-03-12T03:31:54Z

Thanks - I added more clarification regarding the index and made the examples simpler and hopefully clearer.

WillAyd · 2018-03-12T03:36:12Z

pandas/core/generic.py

-            will not be copied recursively, only the reference to the object.
-            This is in contrast to ``copy.deepcopy`` in the Standard Library,
-            which recursively copies object data.
+            With `deep=False` neither the indices nor the data are copied.

        Returns
        -------
        copy : type of caller


Still need to clean up the line that says copy : type of caller. I would explicitly list the objects that could be a caller of this function, which I assume would make this Series or DataFrame but if there's anything else be sure to add that.

Otherwise nice job on all the edits - changes I requested all lgtm

Yes, I would make this "Series or DataFrame"

jorisvandenbossche

Very nice docstring!
Added few more comments

jorisvandenbossche · 2018-03-12T17:09:53Z

pandas/core/generic.py

+
+        Notes
+        -----
+        When `deep=True`, data is copied but actual Python objects


double backticks around deep=True

jorisvandenbossche · 2018-03-12T17:18:05Z

pandas/core/generic.py

-            will not be copied recursively, only the reference to the object.
-            This is in contrast to ``copy.deepcopy`` in the Standard Library,
-            which recursively copies object data.
+            With `deep=False` neither the indices nor the data are copied.

        Returns
        -------
        copy : type of caller


Yes, I would make this "Series or DataFrame"

jorisvandenbossche · 2018-03-12T17:21:20Z

pandas/core/generic.py

+        the calling object's data or index (only references to the data
+        and index are copied). Any changes to the data of the original
+        will be reflected in the shallow copy (and vice versa). Changes
+        to the index will result in a new index as indices are immutable.


I think this line "Changes to the index will result in a new index as indices are immutable" is a bit out of place here, as this is not related to deep=False

For the fact that indices are immutable, I would add a short explanation about that in the Notes section (it is a bit too detailed for the extended summary at the beginning of the docstring).
The main thing is that, even for deep=True, the index data (the underlying numpy array) is not copied, only the object (the Index class holding the array data). This is done for performance reasons: because Index is immutable, it is not needed to copy the data.
(see #19862 for discussion on it)

codecov · 2018-03-12T18:39:45Z

Codecov Report

Merging #20261 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #20261      +/-   ##
==========================================
+ Coverage   91.77%   91.84%   +0.06%     
==========================================
  Files         152      152              
  Lines       49205    49231      +26     
==========================================
+ Hits        45159    45215      +56     
+ Misses       4046     4016      -30

Flag	Coverage Δ
#multiple	`90.22% <100%> (+0.06%)`	⬆️
#single	`41.83% <33.33%> (-0.02%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/generic.py	`95.85% <100%> (ø)`	⬆️
pandas/core/arrays/categorical.py	`96.2% <0%> (-0.02%)`	⬇️
pandas/core/window.py	`96.26% <0%> (-0.01%)`	⬇️
pandas/core/indexes/datetimelike.py	`96.72% <0%> (ø)`	⬆️
pandas/core/panel.py	`97.29% <0%> (ø)`	⬆️
pandas/core/base.py	`96.78% <0%> (ø)`	⬆️
pandas/core/indexes/category.py	`97.3% <0%> (ø)`	⬆️
pandas/core/series.py	`93.84% <0%> (ø)`	⬆️
pandas/core/frame.py	`97.18% <0%> (ø)`	⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7273ea0...18c7be3. Read the comment docs.

jorisvandenbossche · 2018-03-20T12:42:02Z

@thismakessand Do you have time to update this PR based on the remaining feedback ?

thismakessand · 2018-03-20T13:46:13Z

@jorisvandenbossche Yes, it's definitely still on my radar, hoping to find some time later today or tomorrow.

thismakessand · 2018-03-23T02:45:56Z

I read through the linked issues and added the details you mentioned. Looking at the linked issues and other stuff on stack overflow, etc it seems like most of the confusion is around

deep copy not being a "true" deep copy when the series/dataframe contains python objects and
the underlying data of the Index isn't copied even when deep=True.

I think both of these are covered now. Originally I had more details but I was worried going too into the caveats would just be unnecessarily confusing so I cut a bit out. I can add more if it's still unclear.

jorisvandenbossche

Updates are looking good!

jorisvandenbossche · 2018-03-23T08:52:11Z

@thismakessand Thanks a lot!

jorisvandenbossche · 2018-03-23T08:54:23Z

There is one remaining thing: the type description for deep also said 'str'. I removed this for now, as it is not explained, but apparently 'all' is accepted to also copy the Index underlying data.
But will open a new issue for that to see if we actually want to expose this publicly.

DOC: Update pandas.Series.copy docstring

2686324

- Add extended summary - Add examples

jorisvandenbossche added the Docs label Mar 10, 2018

WillAyd requested changes Mar 11, 2018

View reviewed changes

Simplifying examples; adding Notes section

582f047

WillAyd requested changes Mar 11, 2018

View reviewed changes

jreback requested changes Mar 11, 2018

View reviewed changes

Add details regarding handling of index and streamline examples

f1560e1

WillAyd reviewed Mar 12, 2018

View reviewed changes

jorisvandenbossche reviewed Mar 12, 2018

View reviewed changes

Update Returns section to list types of objects

c4aed31

jorisvandenbossche mentioned this pull request Mar 20, 2018

Documation: copy documenation is lacking detail for when deep=False #19505

Closed

Additional details on index handling

541edc2

Update generic.py

18c7be3

jorisvandenbossche approved these changes Mar 23, 2018

View reviewed changes

jorisvandenbossche added this to the 0.23.0 milestone Mar 23, 2018

jorisvandenbossche merged commit 8004eff into pandas-dev:master Mar 23, 2018

javadnoorb pushed a commit to javadnoorb/pandas that referenced this pull request Mar 29, 2018

DOC: Update pandas.Series.copy docstring (pandas-dev#20261)

4a77c96

dworvos pushed a commit to dworvos/pandas that referenced this pull request Apr 2, 2018

DOC: Update pandas.Series.copy docstring (pandas-dev#20261)

18b9bd6

kornilova203 pushed a commit to kornilova203/pandas that referenced this pull request Apr 23, 2018

DOC: Update pandas.Series.copy docstring (pandas-dev#20261)

6e665be

thismakessand deleted the doc-series-copy branch September 29, 2018 02:40


		Shallow copy versus default (deep) copy:

		In a shallow copy the index and data is a reference to the original


		>>> s = pd.Series([1,2], dtype="int32", index=["a", "b"])
		>>> s = pd.Series([1,2], index=["a", "b"])

DOC: Update pandas.Series.copy docstring #20261

DOC: Update pandas.Series.copy docstring #20261

Conversation

thismakessand commented Mar 10, 2018 • edited by jorisvandenbossche Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thismakessand commented Mar 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented Mar 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thismakessand commented Mar 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 12, 2018 • edited Loading

Codecov Report

jorisvandenbossche commented Mar 20, 2018

thismakessand commented Mar 20, 2018

thismakessand commented Mar 23, 2018

jorisvandenbossche left a comment

Choose a reason for hiding this comment

jorisvandenbossche commented Mar 23, 2018

jorisvandenbossche commented Mar 23, 2018

thismakessand commented Mar 10, 2018 •

edited by jorisvandenbossche

Loading

codecov bot commented Mar 12, 2018 •

edited

Loading