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

Fix excel-related docstring errors #24503

Merged
merged 8 commits into from
Dec 31, 2018
Merged

Conversation

thoo
Copy link
Contributor

@thoo thoo commented Dec 30, 2018

Fix errors which are reported by scripts/validate_docstrings.py:

  • pandas.read_excel
  • pandas.io.formats.style.Styler.to_excel
  • Series.to_excel
  • DataFrame.to_excel

* upstream/master:
  DOC: Fixing broken references in the docs (pandas-dev#24497)
  DOC: Splitting api.rst in several files (pandas-dev#24462)
  Fix misdescription in escapechar (pandas-dev#24490)
  Floor and ceil methods during pandas.eval which are provided by numexpr (pandas-dev#24355)
  BUG: Pandas any() returning false with true values present (GH pandas-dev#23070) (pandas-dev#24434)
  Misc separable pieces of pandas-dev#24024 (pandas-dev#24488)
  use capsys.readouterr() as named tuple (pandas-dev#24489)
  REF/TST: replace capture_stderr with pytest capsys fixture (pandas-dev#24496)
  TST- Fixing issue with test_parquet test unexpectedly passing (pandas-dev#24480)
  DOC: Doc build for a single doc made much faster, and clean up (pandas-dev#24428)
  BUG: Fix+test timezone-preservation in DTA.repeat (pandas-dev#24483)
  Implement reductions from pandas-dev#24024 (pandas-dev#24484)
Name Value
0 string1 1
1 string2 2
2 #string3 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this hash on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is for the last example. pd.read_excel('tmp.xlsx', index_col=0, comment='#').

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change from #string3 to #Comment to avoid confusion.

@jreback jreback added Docs IO Excel read_excel, to_excel labels Dec 30, 2018
@jreback
Copy link
Contributor

jreback commented Dec 30, 2018

@datapythonista @WillAyd @gfyoung if any comments.

@jreback jreback added this to the 0.24.0 milestone Dec 30, 2018
@thoo
Copy link
Contributor Author

thoo commented Dec 30, 2018

python make.py --single pandas.read_excel stops building docs after this commit 51c508c (merge upstream)

@jreback
Copy link
Contributor

jreback commented Dec 30, 2018

@thoo, @datapythonista just changed this a bit, can you post what happens?

@thoo
Copy link
Contributor Author

thoo commented Dec 30, 2018

I got errors like /Users/johndoe/pandas/doc/source/index.rst:110: WARNING: toctree references excluded document 'generated/pandas.read_excel' when I run python make.py --single pandas.read_excel.
I think the extensions are not built. When I run python setup.py build_ext --inplace -j 4, I just get running build_ext and no other messages/output as usual.

I just submitted the issue here #24505. Please let me know if you need any other info.

@datapythonista
Copy link
Member

@thoo I just built pandas_excel on master (in Linux):

$ ./doc/make.py html --single=pandas.read_excel
Running Sphinx v1.8.2
making output directory...
[autosummary] generating autosummary for: index.rst
[autosummary] generating autosummary for: /home/mgarcia/src/pandas/doc/source/generated/pandas.read_excel.rst
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 1 source files that are out of date
updating environment: 2 added, 0 changed, 0 removed
reading sources... [100%] index                                                                                                                                                                                                               
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] index                                                                                                                                                                                                                
generating indices... genindex py-modindex
writing additional pages... search
copying static files... done
copying extra files... done
dumping search index in English (code: en) ... done
dumping object inventory... done
build succeeded.

The HTML pages are in doc/build/html.

I think we do a clean before the build, and if I'm not wrong I left the clean of the doc/source/generated/ directory, but may be you can try to remove it manually if it exists.

@datapythonista
Copy link
Member

Ups, sorry, my bad. I didn't have the last changes, looks like building a single API doc is broken after splitting api.rst that has been just merged. Let me take a look...

@thoo
Copy link
Contributor Author

thoo commented Dec 30, 2018

@datapythonista Thanks. I will remove doc/source/generated/.

@datapythonista
Copy link
Member

Opened #24506, which should fix the problem.

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one minor thing, other than that lgtm. Thanks @thoo

* Defaults to ``0`` : 1st sheet as a `DataFrame`
* ``1`` : 2nd sheet as a `DataFrame`
* ``"Sheet1"`` : Load sheet with name "Sheet1"
* ``[0,1,"Sheet5"]`` : Load first, second and sheet named "Sheet5"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add spaces after the commas? Also, unless the formatting changes, I don't think we need the spaces before the colons here (it's required for the parameters, but this is a regular list).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@jreback
Copy link
Contributor

jreback commented Dec 30, 2018

@thoo if you'd pull from master, you should be able to build single pages now.

@thoo
Copy link
Contributor Author

thoo commented Dec 30, 2018

@jreback @datapythonista Thanks. It is working.

@codecov
Copy link

codecov bot commented Dec 30, 2018

Codecov Report

Merging #24503 into master will decrease coverage by 49.25%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24503       +/-   ##
===========================================
- Coverage   92.31%   43.05%   -49.26%     
===========================================
  Files         166      166               
  Lines       52391    52391               
===========================================
- Hits        48363    22557    -25806     
- Misses       4028    29834    +25806
Flag Coverage Δ
#multiple ?
#single 43.05% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/generic.py 39.83% <ø> (-56.79%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-98.65%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
... and 125 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 c4ac0d6...f11bce8. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 30, 2018

Codecov Report

Merging #24503 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24503      +/-   ##
==========================================
- Coverage   31.89%   31.88%   -0.02%     
==========================================
  Files         166      166              
  Lines       52421    52434      +13     
==========================================
- Hits        16722    16718       -4     
- Misses      35699    35716      +17
Flag Coverage Δ
#multiple 30.28% <ø> (-0.01%) ⬇️
#single 31.88% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
pandas/core/generic.py 31.41% <ø> (ø) ⬆️
pandas/core/internals/blocks.py 38.09% <0%> (-0.4%) ⬇️
pandas/core/frame.py 27.82% <0%> (-0.14%) ⬇️
pandas/core/reshape/reshape.py 7.97% <0%> (-0.09%) ⬇️
pandas/core/reshape/pivot.py 8.77% <0%> (ø) ⬆️
pandas/core/computation/expressions.py 58.82% <0%> (ø) ⬆️
pandas/plotting/_style.py 23.65% <0%> (ø) ⬆️
pandas/core/groupby/generic.py 13.71% <0%> (ø) ⬆️
pandas/core/arrays/categorical.py 29.81% <0%> (+0.12%) ⬆️
pandas/core/tools/datetimes.py 27.89% <0%> (+0.29%) ⬆️

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 100ffff...be9651e. Read the comment docs.

* upstream/master:
  REF/TST: replace capture_stdout with pytest capsys fixture (pandas-dev#24501)
  BUG: fix .iat assignment creates a new column (pandas-dev#24495)
  DOC: add checks on the returns section in the docstrings (pandas-dev#23138) (pandas-dev#23432)
  ENH: Add strings_as_fixed_length parameter for df.to_records() (pandas-dev#18146) (pandas-dev#22229)
  TST: Skip db tests unless explicitly specified in -m pattern (pandas-dev#24492)
  Mix EA into DTA/TDA; part of 24024 (pandas-dev#24502)
  DOC: Fix building of a single API document (pandas-dev#24506)
@thoo
Copy link
Contributor Author

thoo commented Dec 30, 2018

It looks like pytest pandas/tests/io/test_excel.py doesn't pass on master. So hopefully that is not related to this PR.

@datapythonista
Copy link
Member

there are some tests that check the content of the docstrings, so it's probably related to it, I'd take a look

@jreback jreback merged commit e4cd00b into pandas-dev:master Dec 31, 2018
@jreback
Copy link
Contributor

jreback commented Dec 31, 2018

thanks @thoo

@thoo thoo deleted the read_excel-docstring branch January 2, 2019 20:25
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs IO Excel read_excel, to_excel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: Fix docstrings of Excel related functions
4 participants