DOC: update the `to_pickle` & `read_pickle` docstring #20253

minggli · 2018-03-10T22:13:34Z

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.DataFrame.to_pickle) ####################
################################################################################

Pickle (serialize) object to input file path.

Parameters
----------
path : str
    File path where the pickled object will be stored.
compression : {'infer', 'gzip', 'bz2', 'xz', None}, default 'infer'
    A string representing the compression to use in the output file. By
    default, infers from the file extension in specified path.

    .. versionadded:: 0.20.0
protocol : int
    Int which indicates which protocol should be used by the pickler,
    default HIGHEST_PROTOCOL (see [1], paragraph 12.1.2). The possible
    values for this parameter depend on the version of Python. For
    Python 2.x, possible values are 0, 1, 2. For Python>=3.0, 3 is a
    valid value. For Python >= 3.4, 4 is a valid value. A negative
    value for the protocol parameter is equivalent to setting its value
    to HIGHEST_PROTOCOL.

    .. [1] https://docs.python.org/3/library/pickle.html
    .. versionadded:: 0.21.0

See Also
--------
read_pickle : Load pickled pandas object (or any object) from file.
DataFrame.to_hdf : Write the contained data to an HDF5 file using
    HDFStore.
DataFrame.to_sql : Write records stored in a DataFrame to a SQL
    database.
DataFrame.to_parquet : Write a DataFrame to the binary parquet format.

Examples
--------
>>> original_df = pd.DataFrame({"foo": range(5), "bar": range(5, 10)})
>>> original_df
   foo  bar
0    0    5
1    1    6
2    2    7
3    3    8
4    4    9
>>> original_df.to_pickle("./dummy.pkl")

>>> unpickled_df = pd.read_pickle("./dummy.pkl")
>>> unpickled_df
   foo  bar
0    0    5
1    1    6
2    2    7
3    3    8
4    4    9

>>> import os
>>> os.remove("./dummy.pkl")

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

Errors found:
	No extended summary found
	Errors in parameters section
		Parameter "compression" description should finish with "."
		Parameter "protocol" description should finish with "."
	No returns section found

description already finishes with "."
.. versionadded:: tag should not end with "."

jreback · 2018-03-10T22:26:31Z

pandas/io/pickle.py

-
+    Examples
+    --------
+    >>> from pandas import DataFrame, read_pickle


you don't need these imports, instead use pd.DataFrame and pd.read_pickle

jreback · 2018-03-10T22:27:26Z

pandas/io/pickle.py

+    >>> original_df
+       foo  bar
+    0    0    5
+    1    1    6


can you add a See Also and ref read_pickle (to_hdf, to_sql, to_parquet) also good

jreback · 2018-03-10T22:27:55Z

pandas/io/pickle.py

@@ -72,6 +93,28 @@ def read_pickle(path, compression='infer'):
    Returns
    -------
    unpickled : type of object stored in file


add a See Also and ref DataFrame.to_pickle, pd.read_hdf, pd.read_sql, pd.read_parquet

jreback · 2018-03-11T13:23:09Z

pandas/core/generic.py

+        for the protocol parameter is equivalent to setting its value to
+        HIGHEST_PROTOCOL.
+
+        .. [1] https://docs.python.org/3/library/pickle.html


I think we are putting these in a Reference section @jorisvandenbossche ?

That's the numpydoc way yes, but currently we have no docstring using that, so I would leave it like this for now

jreback · 2018-03-11T13:24:40Z

pandas/io/pickle.py

+    --------
+    pandas.DataFrame.to_pickle
+    pandas.Series.to_pickle
+    pandas.read_hdf,


don't put commas at the end

I think we add text describing each of these

jreback · 2018-03-11T13:25:00Z

pandas/io/pickle.py

+    See Also
+    --------
+    pandas.read_pickle
+    pandas.to_hdf


jreback · 2018-03-11T13:25:23Z

pandas/core/generic.py

+
+    See Also
+    --------
+    pandas.read_pickle


can you add similar to what you adding in other See Also below

jorisvandenbossche

Thanks for the PR!

I am not sure that I find it worth to use a shared_doc system here. It's used only for substituting Series/DataFrame in the summary line and path parameter explanation. I don't think that's worth the added complexity. I would rather make the docstring generic for both. It's not that the Series version has different options or behaviours.

jorisvandenbossche · 2018-03-12T07:39:10Z

pandas/core/generic.py

+        default HIGHEST_PROTOCOL (see [1], paragraph 12.1.2). The possible
+        values for this parameter depend on the version of Python. For
+        Python 2.x, possible values are 0, 1, 2. For Python>=3.0, 3 is a
+        valid value. For Python >= 3.4, 4 is a valid value.A negative value


space after '.'

jorisvandenbossche · 2018-03-12T07:50:17Z

pandas/core/generic.py

+
+    See Also
+    --------
+    pandas.read_pickle : Load pickled pandas object (or any other pickled


You can leave out the 'pandas' in all of those references

removed 👍

jorisvandenbossche · 2018-03-12T07:52:45Z

pandas/core/generic.py

+        for the protocol parameter is equivalent to setting its value to
+        HIGHEST_PROTOCOL.
+
+        .. [1] https://docs.python.org/3/library/pickle.html


That's the numpydoc way yes, but currently we have no docstring using that, so I would leave it like this for now

jorisvandenbossche · 2018-03-12T07:53:00Z

pandas/core/generic.py

+    path : string
+        File path where the pickled %(klass)s object will be stored.
+    compression : {'infer', 'gzip', 'bz2', 'xz', None}, default 'infer'
+        A string representing the compression to use in the output file.


Can you add that the default 'infer' infers it from the specified path?

added. 💯

codecov · 2018-03-12T16:07:39Z

Codecov Report

❗ No coverage uploaded for pull request base (master@fb556ed). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #20253   +/-   ##
=========================================
  Coverage          ?   91.76%           
=========================================
  Files             ?      150           
  Lines             ?    49148           
  Branches          ?        0           
=========================================
  Hits              ?    45099           
  Misses            ?     4049           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.14% <100%> (?)`
#single	`41.9% <100%> (?)`

Impacted Files	Coverage Δ
pandas/core/generic.py	`95.85% <100%> (ø)`

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 fb556ed...26b3e2e. Read the comment docs.

minggli · 2018-03-12T21:49:53Z

@jorisvandenbossche thanks for the comments. applied changes including reverting to generic docstring. agree that for the benefits, not necessary to use _shared_docs.

jreback · 2018-03-13T10:04:15Z

pandas/core/generic.py

+        4    4    9
+        >>> original_df.to_pickle("./dummy.pkl")
+
+        >>> unpickled_df = pd.read_pickle("./dummy.pkl")


@jorisvandenbossche how handling file paths in doc-strings?

@jreback commented about it (using os.remove to remove remaining file), issue to discuss this is here: #20302

great, see that now.

thanks guys for looking into it.

jreback · 2018-03-13T10:04:52Z

lgtm. @jorisvandenbossche @datapythonista

jorisvandenbossche

Thanks for the updates! Looking good

Added some minor additional comments. One more general comment: it might be nice to add an extended summary very briefly explaining what "pickle" is and linking to the python docs about it.

jorisvandenbossche · 2018-03-13T10:07:25Z

pandas/core/generic.py

+        2    2    7
+        3    3    8
+        4    4    9
+


Can you add here a

>>> import os >>> os.remove("./dummy.pkl")

jorisvandenbossche · 2018-03-13T10:08:51Z

pandas/core/generic.py

+        See Also
+        --------
+        read_pickle : Load pickled pandas object (or any other pickled object)
+                      from the specified file path.


Can you only indent this line with 4 spaces? Like

read_pickle : Load pickled pandas object (or any other pickled object) from the specified file path.

Both work for sphinx, but we mainly use this pattern, so it's better to be consistent in this.

(same for the other ones below)

jorisvandenbossche · 2018-03-13T10:11:34Z

pandas/io/pickle.py

@@ -52,15 +81,17 @@ def to_pickle(obj, path, compression='infer', protocol=pkl.HIGHEST_PROTOCOL):
 def read_pickle(path, compression='infer'):
    """
    Load pickled pandas object (or any other pickled object) from the specified
-    file path
+    file path.


Can you try to fit this on one line?

Maybe the "or any other pickled object" is not needed in the summary line and can go in an extended summary, as the typical use case should be pandas objects.
Or, maybe the "from specified file path" can be shortened to "from file."

datapythonista

Looks good, added some comments.

datapythonista · 2018-03-13T15:08:39Z

pandas/core/generic.py

@@ -1906,23 +1906,54 @@ def to_pickle(self, path, compression='infer',
        Parameters
        ----------
        path : string


str instead of string

datapythonista · 2018-03-13T15:09:39Z

pandas/core/generic.py

        compression : {'infer', 'gzip', 'bz2', 'xz', None}, default 'infer'
-            a string representing the compression to use in the output file
+            A string representing the compression to use in the output file. By
+            default, infers from the specified path.


"infers from the specified file extension" may be?

yes pythonista.

datapythonista · 2018-03-13T15:10:26Z

pandas/core/generic.py

-            HIGHEST_PROTOCOL.
+            valid value. For Python >= 3.4, 4 is a valid value. A negative
+            value for the protocol parameter is equivalent to setting its value
+            to HIGHEST_PROTOCOL.

            .. [1] https://docs.python.org/3/library/pickle.html


I think is more standard to have this in a References section.

@jorisvandenbossche your opinion?

I previously said it was fine, because we don't use References sections in many cases (most of the time we use inline links), another thing we can discuss in further improving the guidelines.

datapythonista · 2018-03-13T15:11:52Z

pandas/io/pickle.py

+    3    3    8
+    4    4    9
+
+    See Also


See Also should go before the Examples.

Did you push your latest changes?

Just because you mentioned you corrected it, but the see also is still after the examples?

oh sorry, just missed this one and did others.

jorisvandenbossche

Added some remaining comment about the "See Also" descriptions.
Looking good for the rest!

jorisvandenbossche · 2018-03-13T21:49:45Z

pandas/io/pickle.py

+    See Also
+    --------
+    read_pickle : Load pickled pandas object (or any object) from file.
+    DataFrame.to_hdf : Write the contained data to an HDF5 file using HDFStore.


I would make this a bit simpler (eg no need to mention HDFStore): "Write DataFame to HDF5 file"

amended 👍

jorisvandenbossche · 2018-03-13T21:50:14Z

pandas/io/pickle.py

+    --------
+    read_pickle : Load pickled pandas object (or any object) from file.
+    DataFrame.to_hdf : Write the contained data to an HDF5 file using HDFStore.
+    DataFrame.to_sql : Write records stored in a DataFrame to a SQL database.


To be a bit more consistent in the wording here, maybe just "Write DataFrame to a SQL database" ?

amended 👍

jorisvandenbossche · 2018-03-13T21:51:50Z

pandas/io/pickle.py

+    DataFrame.to_pickle : Pickle (serialize) DataFrame object to input file
+        path.
+    Series.to_pickle : Pickle (serialize) Series object to input file path.
+    read_hdf : read from the store, close it if we opened it.


I think the explanation about closing the file is too detailed for the See Also explanation. Maybe something like "Read HDF5 file into a DataFrame" ?

amended 👍

jorisvandenbossche · 2018-03-13T21:52:18Z

pandas/io/pickle.py

+    Series.to_pickle : Pickle (serialize) Series object to input file path.
+    read_hdf : read from the store, close it if we opened it.
+    read_sql : Read SQL query or database table into a DataFrame.
+    read_parquet : Load a parquet object from the file path, returning a


Similar comment here, "from the file path" is rather specific, would leave out that part

amended 👍

jorisvandenbossche · 2018-03-13T21:53:28Z

pandas/io/pickle.py

@@ -10,15 +10,17 @@

 def to_pickle(obj, path, compression='infer', protocol=pkl.HIGHEST_PROTOCOL):
    """
-    Pickle (serialize) object to input file path
+    Pickle (serialize) object to input file path.


I find it a bit strange to say we pickle the object the a "file path", it's actually to a file (located at the file path), so maybe simplify to "Pickle (serialize) object to file." ?

amended 👍

jreback · 2018-03-13T22:56:25Z

this lgtm. do we use Reference in other doc-strings?

jorisvandenbossche · 2018-03-14T08:32:18Z

@minggli Thanks a lot for the PR!

minggli added 10 commits March 10, 2018 20:27

update docstring and add example

3d6ed4a

update docstring and add example

62a202f

add space

03fa85b

DataFrame.to_pickle docstring

3169811

Series.to_pickle docstring

d014e97

add to_pickle to _shared_docs

202f411

move quote

05e1bce

remove blank line

f674845

miscellaneous fixes

556adf4

miscellaneous fixes

42fcc03

minggli changed the title ~~DOC: update the to_pickle & 'read_pickle' docstring~~ DOC: update the to_pickle & read_pickle docstring Mar 10, 2018

jreback requested changes Mar 10, 2018

View reviewed changes

jreback added Docs IO Data IO issues that don't fit into a more specific label labels Mar 10, 2018

minggli added 5 commits March 10, 2018 23:00

remove import and add See Also

e69ea5c

remove import and add See Also

f36c6dd

add more See Also

5f152ed

use proper warning with embedded hyperlink

c6231b0

remove pandas.to_pickle from See Also

0c3a442

jreback requested changes Mar 11, 2018

View reviewed changes

minggli added 6 commits March 11, 2018 13:31

remove commas in See Also

709ca74

additional output in See Also

c15d454

add descriptions in See Also references

b3d9cee

add descriptions in See Also references

ef19c93

correct references and indentation

33a9b1f

correct indentation

7be8f3b

minggli closed this Mar 12, 2018

minggli reopened this Mar 12, 2018

jorisvandenbossche reviewed Mar 12, 2018

View reviewed changes

minggli added 4 commits March 12, 2018 21:26

revert frame

d69c73f

revert series

e2af5a3

remove shared_doc, pandas. and add infer description

46b7342

remove pandas. and add infer description

7f1d3d4

jorisvandenbossche mentioned this pull request Mar 12, 2018

DOC: update the pickle.py docstring #20106

Closed

9 tasks

minggli closed this Mar 13, 2018

minggli reopened this Mar 13, 2018

jreback approved these changes Mar 13, 2018

View reviewed changes

jreback added this to the 0.23.0 milestone Mar 13, 2018

jorisvandenbossche reviewed Mar 13, 2018

View reviewed changes

datapythonista reviewed Mar 13, 2018

View reviewed changes

minggli added 3 commits March 13, 2018 18:46

miscellaneous changes

3e545f3

miscellaneous changes

c1d6f03

move See Also before Example and add os.remove

39969d5

jorisvandenbossche reviewed Mar 13, 2018

View reviewed changes

minggli added 2 commits March 13, 2018 22:03

simplify See Also and to_pickle summary.

c1a9d57

simplify See Also and to_pickle summary in pandas.io.pickle

26b3e2e

jorisvandenbossche approved these changes Mar 14, 2018

View reviewed changes

jorisvandenbossche merged commit 12de69b into pandas-dev:master Mar 14, 2018

minggli deleted the doc/to_pickle branch March 14, 2018 09:13

+2    7
+3    8
+4    9

DOC: update the to_pickle & read_pickle docstring #20253

DOC: update the to_pickle & read_pickle docstring #20253

Conversation

minggli commented Mar 10, 2018 • edited 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

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

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

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

minggli commented Mar 12, 2018 • edited 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

jreback commented Mar 13, 2018

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista 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

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

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

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

jreback commented Mar 13, 2018

jorisvandenbossche commented Mar 14, 2018

DOC: update the `to_pickle` & `read_pickle` docstring #20253

DOC: update the `to_pickle` & `read_pickle` docstring #20253

minggli commented Mar 10, 2018 •

edited

Loading

codecov bot commented Mar 12, 2018 •

edited

Loading

minggli commented Mar 12, 2018 •

edited

Loading