DOC: Fix to_latex docstring. #22516

Moisan · 2018-08-26T19:32:47Z

tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff

Fix the DataFrame.to_latex docstring to match scripts/validate_docstrings.py as explained in #22459 and add an example.

The docstring was previously in a variable that was only used in to_latex. I put it in the method docstring instead. The @Substitution wasn't matching anything, I suspect this dates back to the common docstring in io/formats/format.py.

codecov · 2018-08-26T21:21:33Z

Codecov Report

Merging #22516 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #22516      +/-   ##
==========================================
- Coverage   92.04%   92.04%   -0.01%     
==========================================
  Files         169      169              
  Lines       50787    50784       -3     
==========================================
- Hits        46746    46743       -3     
  Misses       4041     4041

Flag	Coverage Δ
#multiple	`90.45% <100%> (-0.01%)`	⬇️
#single	`42.29% <100%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/generic.py	`96.43% <100%> (-0.01%)`	⬇️
pandas/core/indexes/multi.py	`95.41% <0%> (ø)`	⬆️
pandas/core/frame.py	`97.2% <0%> (ø)`	⬆️
pandas/core/indexes/base.py	`96.45% <0%> (ø)`	⬆️

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 fa47b8d...c64fb5d. Read the comment docs.

datapythonista

Great contribution, the docstring really needed some care. Added just some minor comments mainly about the parameter types.

datapythonista · 2018-08-27T12:53:10Z

pandas/core/generic.py

+        Parameters
+        ----------
+        buf : StringIO-like, optional
+            Buffer to write to.


More than StringIO-like I'd say this is a file descriptor. I guess as other methods, if None, the output is returned as a string. I think in this cases we usually say file descriptor or None, instead of optional, but in either case, we want to explain that if None it returns the string.

datapythonista · 2018-08-27T12:54:46Z

pandas/core/generic.py

+        ----------
+        buf : StringIO-like, optional
+            Buffer to write to.
+        columns : sequence, optional, default None


No ened for default None when it's optional. I think we agreed on using label when talking about the objects in the indices, so it could make sense to have the type as list of label.

datapythonista · 2018-08-27T12:56:29Z

pandas/core/generic.py

+            Write row names (index).
+        na_rep : str, default 'NaN'
+            Missing data representation.
+        formatters : list or dict of one-param. functions, optional


The type is a bit confusing. Don't know what it's expected, would it be list of function or dict of {str: function}?

datapythonista · 2018-08-27T12:58:09Z

pandas/core/generic.py

+            Formatter functions to apply to columns' elements by position or
+            name. The result of each function must be a unicode string.
+            List must be of length equal to the number of columns.
+        float_format : str, default None


optional instead of default None would make more sense in this case to me. It's not always clear, but in general we use default None when the None value is used as None. When None means the feature is not used, it's optional.

datapythonista · 2018-08-27T12:59:26Z

pandas/core/generic.py

+            Format string for floating point numbers.
+        sparsify : bool, optional, default None
+            Set to False for a DataFrame with a hierarchical index to print
+            every multiindex key at each row.


Do you know if None and False are the same here? if that's the case I'd prefer to get rid of optional and explain in the description.

They are not the same, in formats.py:

if sparsify is None: sparsify = get_option("display.multi_sparse")

datapythonista · 2018-08-27T12:59:45Z

pandas/core/generic.py

+        sparsify : bool, optional, default None
+            Set to False for a DataFrame with a hierarchical index to print
+            every multiindex key at each row.
+        index_names : bool, optional, default True


unnecessary optional

datapythonista · 2018-08-27T13:00:00Z

pandas/core/generic.py

+            Set to False for a DataFrame with a hierarchical index to print
+            every multiindex key at each row.
+        index_names : bool, optional, default True
+            Prints the names of the indexes.
        bold_rows : boolean, default False


bool instead of boolean

datapythonista · 2018-08-27T13:01:42Z

pandas/core/generic.py

+            When set to None, the value will default from the pandas config
+            module. Use a longtable environment instead of tabular. Requires
+            adding a \usepackage{longtable} to your LaTeX preamble.
+        escape : bool, default will be read from the pandas config module


can you leave here simple default None, and explain about the config in the description?

datapythonista · 2018-08-27T13:04:09Z

pandas/core/generic.py

+        See Also
+        --------
+        DataFrame.to_csv : Write a DataFrame to CSV format.
+        DataFrame.to_excel : Write a DataFrame to an Excel file.


I'd have to_string and to_html in this case, which to me are conceptually more similar than to_csv or to_excel (formatting to present vs formatting to export).

jeffyancey · 2018-09-03T06:13:46Z

pandas/core/generic.py

-            Default: True.
+            <https://en.wikibooks.org/wiki/LaTeX/Tables>`__ e.g. 'rcl' for 3
+            columns.
+        longtable : bool, default None


should it be bool, optional, default None, like on line 2558?

In this case it should be bool, optional. We never use whatever_type, optional, default None. We use only one of them.

When the None default value is being used, we use default None, imagine for example .fillna(value=None) where the None is the value used to impute.

When the None is just a flag, then we use optional. For example, in this case the longtable won't get the value None itself, but a value from the config. Meaning that it's optional to provide a longtable value, as we can use that.

jeffyancey · 2018-09-03T06:14:31Z

pandas/core/generic.py

+            When set to None, the value will default from the pandas config
+            module. Use a longtable environment instead of tabular. Requires
+            adding a \usepackage{longtable} to your LaTeX preamble.
+        escape : bool, default None


same question as above.

… add doc for column_format defaults

pep8speaks · 2018-09-04T17:36:24Z

Hello @Moisan! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/generic.py !

datapythonista

Can you run ./scripts/validate_docstrings.py pandas.DataFrame.to_latex?

I think it will complain that the first line should be a single line (this is important for the page with the list of methods)

When the script says everything is all right, I'm happy with it. Really nice change, much better docstring now.

Moisan · 2018-09-05T13:22:16Z

./scripts/validate_docstrings.py pandas.DataFrame.to_latex returns that everything is correct.

datapythonista · 2018-09-05T14:47:07Z

That's an error in the script them. Can you post the output of the script to see how the docstring is veing rendered, and see if it helps to see what's wrong with the script.

And after that fix the docstring, so the first line (short summary) fits in a single line, as described here: https://pandas.pydata.org/pandas-docs/stable/contributing_docstring.html

Moisan · 2018-09-05T18:23:30Z

Here is the output. I can open an issue regarding the bug of scripts/validate_docstrings.py.

datapythonista

lgtm, thanks for the contribution @Moisan

If you can open the issue for the validation, that would be great.

jreback · 2018-09-08T03:10:37Z

thanks @Moisan

DOC: Fix to_latex docstring.

fadf5e7

gfyoung added Docs IO LaTeX to_latex labels Aug 27, 2018

gfyoung requested a review from datapythonista August 27, 2018 09:46

datapythonista reviewed Aug 27, 2018

View reviewed changes

Improve to_latex docstring after PR comments

8d59ef4

jeffyancey reviewed Sep 3, 2018

View reviewed changes

Use optional instead of 'default None' when None is just a flag. Also…

052a32f

… add doc for column_format defaults

datapythonista reviewed Sep 4, 2018

View reviewed changes

Add a short summary to DataFrame.to_latex docstring

c64fb5d

datapythonista approved these changes Sep 5, 2018

View reviewed changes

Moisan mentioned this pull request Sep 5, 2018

DOC: scripts/validate_docstrings.py should detect when the short summary span over multiple lines #22615

Closed

jreback added this to the 0.24.0 milestone Sep 8, 2018

jreback merged commit 996f361 into pandas-dev:master Sep 8, 2018

aeltanawy pushed a commit to aeltanawy/pandas that referenced this pull request Sep 20, 2018

DOC: Fix to_latex docstring. (pandas-dev#22516)

338683e

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

DOC: Fix to_latex docstring. (pandas-dev#22516)

58e7d20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Fix to_latex docstring. #22516

DOC: Fix to_latex docstring. #22516

Moisan commented Aug 26, 2018 •

edited

Loading

codecov bot commented Aug 26, 2018 •

edited

Loading

datapythonista left a comment

datapythonista Aug 27, 2018

datapythonista Aug 27, 2018

datapythonista Aug 27, 2018

datapythonista Aug 27, 2018

datapythonista Aug 27, 2018

Moisan Aug 28, 2018

datapythonista Aug 27, 2018

datapythonista Aug 27, 2018

datapythonista Aug 27, 2018

datapythonista Aug 27, 2018

jeffyancey Sep 3, 2018

datapythonista Sep 3, 2018

jeffyancey Sep 3, 2018

pep8speaks commented Sep 4, 2018

datapythonista left a comment

Moisan commented Sep 5, 2018

datapythonista commented Sep 5, 2018

Moisan commented Sep 5, 2018 •

edited

Loading

datapythonista left a comment

jreback commented Sep 8, 2018

DOC: Fix to_latex docstring. #22516

DOC: Fix to_latex docstring. #22516

Conversation

Moisan commented Aug 26, 2018 • edited Loading

codecov bot commented Aug 26, 2018 • edited Loading

Codecov Report

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

pep8speaks commented Sep 4, 2018

datapythonista left a comment

Choose a reason for hiding this comment

Moisan commented Sep 5, 2018

datapythonista commented Sep 5, 2018

Moisan commented Sep 5, 2018 • edited Loading

datapythonista left a comment

Choose a reason for hiding this comment

jreback commented Sep 8, 2018

Moisan commented Aug 26, 2018 •

edited

Loading

codecov bot commented Aug 26, 2018 •

edited

Loading

Moisan commented Sep 5, 2018 •

edited

Loading