DOC: Fixes to docstrings, mainly PR09 errors #29530

HughKelley · 2019-11-10T14:48:17Z

closes #28602

pep8speaks · 2019-11-10T14:48:37Z

Hello @HughKelley! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-12-31 19:55:57 UTC

jreback · 2019-11-11T14:03:54Z

does this completely close #28602?

HughKelley · 2019-11-11T14:18:33Z

does this completely close #28602?

yes, after merging, running python ./scripts/validate_docstrings.py --errors=PR09 should not return any errors and we can add that error to the CI checks (Which I'd like to try to make a PR for if it's alright).

HughKelley · 2019-11-11T14:19:23Z

hit the wrong button in the comment dialogue and closed by accident

datapythonista · 2019-11-11T16:30:24Z

@HughKelley in ci/code_checks.sh we validate that errors in docstrings don't have. Can you add PR09 to the list of errors we check, if this is the last batch. This way future changes to docstrings won't add new PR09 errors (the CI will fail).

datapythonista · 2019-11-11T16:31:41Z

Ups, just seen your previous comment now. Please better add the check here.

jorisvandenbossche

Thanks for working on this! I think a few of the changes in the indentation of "deprecated" is not correct, for the rest looks good!

jorisvandenbossche · 2019-11-12T08:37:42Z

pandas/io/excel/_base.py

@@ -82,7 +82,7 @@
    * If None, then parse all columns.
    * If int, then indicates last column to be parsed.

-      .. deprecated:: 0.24.0
+    .. deprecated:: 0.24.0


I don't think this change is fully correct, as the "deprecated" notion is specific to this bullet item, not to the general keyword

(the same might be true for the other occurrences here)

Hi, thanks for reviewing. While i agree that it would be better to have the warning indented, that breaks the validation, raising error PR09 description should end in . I have experimented with different amounts of indentation but this is the only one that doesn't raise a warning.

The way the docs render without the two spaces, seen below, I think are sufficiently clear about what the warning is referring to but I'm open to other solutions. I just haven't found any.

While i agree that it would be better to have the warning indented, that breaks the validation, raising error PR09 description should end in .

Then I would consider that a bug in the validation script that could be fixed instead?

maybe @datapythonista can speak to exactly what is being worked on in the checks, I know there are some changes in the works to handle bulleted lists without leading text.

datapythonista · 2019-11-12T16:10:21Z

That's a false positive, and I think it's fixed in the numpydoc version of our script. I expect to have the PR that replaces our script by the numpydoc version in few days, so don't worry about those errors, you can leave the docstrings correct.

…_batch_5

jorisvandenbossche · 2019-11-19T13:48:11Z

@HughKelley seems something went wrong with your latest update with master (there are too many commits included here, giving a big diff).

Normally something like

git checkout PR09_batch_5
git fetch upstream
git merge upstream/master
git push origin PR09_batch_5

should do the trick

HughKelley · 2019-11-19T14:00:34Z

something went wrong with your latest update with master

definitely. I used rebase instead of merge.

I think I'm going to close this PR since it's a mess, cherrypick what I need into a new branch and open a new PR once the CI checks are all passing.

Unless you'd rather I do something else.

Thanks for the help/patience as I figure this stuff out.

jorisvandenbossche · 2019-11-19T14:16:30Z

I think that the snippet I posted should still be able to clean it up (even after the rebase), but if that doesn't work out, no problem in creating a clean branch / PR!
(BTW, you can actually create a new branch locally but still push to this remote branch name / update this PR, you only then need to force push)

datapythonista · 2019-11-19T14:35:16Z

The first section in this post describes what I'd do: https://datapythonista.github.io/blog/useful-git-commands.html

…09_batch_5

HughKelley · 2019-12-11T14:44:53Z

@datapythonista @jorisvandenbossche these checks are green.

Joris, the indentation error you pointed out is fixed.

Am I responsible for resolving the merge conflict? Should I update the PR with the most recent changes to master and let the checks run again?

Thanks!

datapythonista · 2019-12-11T14:52:58Z

Yes, you need to resolve the conflicts before we can merge. git fetch upstream and git merge upstream/master should get the conflicts locally, so you can fix them. Thanks!

HughKelley · 2019-12-11T15:10:43Z

will do, thanks

datapythonista

Looks good, just couple of comments.

datapythonista · 2019-12-11T15:47:15Z

ci/code_checks.sh

@@ -286,7 +286,7 @@ fi
 ### DOCSTRINGS ###
 if [[ -z "$CHECK" || "$CHECK" == "docstrings" ]]; then

-    MSG='Validate docstrings (GL03, GL04, GL05, GL06, GL07, GL09, GL10, SS04, SS05, PR03, PR04, PR05, PR10, EX04, RT01, RT04, RT05, SA01, SA02, SA03, SA05)' ; echo $MSG
+    MSG='Validate docstrings (GL03, GL04, GL05, GL06, GL07, GL09, GL10, SS04, SS05, PR03, PR04, PR05, PR09, PR10, EX04, RT01, RT04, RT05, SA01, SA02, SA03, SA05)' ; echo $MSG
    $BASE_DIR/scripts/validate_docstrings.py --format=azure --errors=GL03,GL04,GL05,GL06,GL07,GL09,GL10,SS04,SS05,PR03,PR04,PR05,PR10,EX04,RT01,RT04,RT05,SA01,SA02,SA03,SA05


you should add the error you fixed here too, the previous is only a message for the log

datapythonista · 2019-12-11T15:48:53Z

pandas/core/frame.py


-                .. versionadded:: 0.24.0
+              .. versionadded:: 0.24.0


did you check if the html version of this docstring looks good? those indentations are always tricky

This is correct (it has the same indentation as the list item)

HughKelley · 2019-12-11T18:06:28Z

@datapythonista thanks for reviewing. The checks do not pass as there are still PR09 errors in the code.

Has the update to the numpydoc script occured yet? Is there an open issue or PR for that I can keep an eye on?

The script doesn't recognize the . at the end of the description of method in pandas.DataFrame.corr.

datapythonista · 2019-12-12T05:49:33Z

I've been working in using the numpydoc script, but there is still work to do, and I'm travelling, so it'll take some rhinestone to be ready.

We can merge this in the meantime, but you should undo the changes to the comment in code_checks. sh.

Also, please have a look at what is broken in the CI, so we can merge.

Thanks!

jorisvandenbossche · 2019-12-12T07:20:15Z

pandas/core/frame.py

@@ -7140,17 +7142,16 @@ def corr(self, method="pearson", min_periods=1):
        Parameters
        ----------
        method : {'pearson', 'kendall', 'spearman'} or callable
-            Method of correlation:
-
+            Method of correlation.


You need to keep the blank line between the first sentence and the bullet points (I think that's the reason sphinx is complaining).

BTW, I think the colon (:) also makes sense to keep before a bullet point list.

yup, thanks. That was just me playing with it to see if I could get the validation to accept it

datapythonista · 2019-12-29T14:05:06Z

@HughKelley do you have time to address the last comments, so we can get this merged? Thanks!

HughKelley · 2019-12-31T21:09:43Z

@HughKelley do you have time to address the last comments, so we can get this merged? Thanks!

@datapythonista

Thanks for following up. Sorry this wasn't more of a priority for me during the holidays.

i'm actually cherry picking stuff into a new (cleaner) PR that I'll get merged in the new year.

thanks very much for everyone's work on the package and helping me as i learn this stuff.

HughKelley added 13 commits November 10, 2019 08:35

PR09 in pandas.DataFrame.idxmax

fb76a17

PR09 in pandas.DataFrame.idxmin

0f40afb

PR09 in pandas.DataFrame.sort_values

94ac074

PR09 in pandas.DataFrame.to_html

e8a5ab1

PR09 in pandas.arrays.DatetimeArray

90cc836

PR09 in pandas.Categorical

9e6d290

PR09 in pandas.Grouper

64b7c75

PR09 in pandas.ExcelWriter

d84b2a1

PR09 in pandas.core.window.ewm.EWM.cov

214add3

PR09 in pandas.core.groupby.GroupBy.rank

705f441

PR09 in pandas.PeriodIndex

b3f1074

PR09 in pandas.PeriodIndex.asfreq

9d7f405

PR09 in pandas.PeriodIndex.to_timestamp

cb524fd

HughKelley added 3 commits November 10, 2019 09:49

whitespace

9a517aa

PR09 in pandas.DataFrame.corr

b99d417

PR09 in pandas.read_excel

8095d30

jreback added the Docs label Nov 11, 2019

jreback added this to the 1.0 milestone Nov 11, 2019

HughKelley closed this Nov 11, 2019

HughKelley reopened this Nov 11, 2019

datapythonista changed the title ~~Pr09 batch 5~~ DOC: Fixes to docstrings, mainly PR09 errors Nov 11, 2019

jorisvandenbossche requested changes Nov 12, 2019

View reviewed changes

Add PR09 to CI checks

e8c4c50

HughKelley added 8 commits November 18, 2019 11:45

PR09 in pandas.read_excel

1fd931a

Add PR09 to CI checks

a5eb1a9

fix spacing

3ff70b4

fix indentation of deprecation warning

080a77b

indentation in DataFrame.corr

de22262

indentation in pandas.read_excel

c56b0f9

indentation in read_excel

18f492e

Merge branch 'PR09_batch_5' of github.com:HughKelley/pandas into PR09…

0826a6f

…_batch_5

HughKelley added 2 commits November 20, 2019 11:47

Merge branch 'master' of https://github.com/pandas-dev/pandas into PR…

25167fb

…09_batch_5

merge master

c4848eb

merge master resolve conflict

0f9c6d1

datapythonista reviewed Dec 11, 2019

View reviewed changes

merge master

2f72294

PR09 errors

d434831

jorisvandenbossche reviewed Dec 12, 2019

View reviewed changes

merge master

fdc670e

HughKelley closed this Dec 31, 2019

HughKelley mentioned this pull request Jan 2, 2020

DOC: Fixing PR09 formatting errors #30622

Merged

2 tasks

HughKelley deleted the PR09_batch_5 branch January 6, 2020 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Fixes to docstrings, mainly PR09 errors #29530

DOC: Fixes to docstrings, mainly PR09 errors #29530

HughKelley commented Nov 10, 2019 •

edited

Loading

pep8speaks commented Nov 10, 2019 •

edited

Loading

jreback commented Nov 11, 2019

HughKelley commented Nov 11, 2019

HughKelley commented Nov 11, 2019

datapythonista commented Nov 11, 2019

datapythonista commented Nov 11, 2019

jorisvandenbossche left a comment

jorisvandenbossche Nov 12, 2019

HughKelley Nov 12, 2019

jorisvandenbossche Nov 12, 2019

HughKelley Nov 12, 2019

datapythonista commented Nov 12, 2019

jorisvandenbossche commented Nov 19, 2019 •

edited

Loading

HughKelley commented Nov 19, 2019

jorisvandenbossche commented Nov 19, 2019 •

edited

Loading

datapythonista commented Nov 19, 2019

HughKelley commented Dec 11, 2019

datapythonista commented Dec 11, 2019

HughKelley commented Dec 11, 2019

datapythonista left a comment

datapythonista Dec 11, 2019

datapythonista Dec 11, 2019

jorisvandenbossche Dec 11, 2019

HughKelley commented Dec 11, 2019

datapythonista commented Dec 12, 2019

jorisvandenbossche Dec 12, 2019

HughKelley Dec 12, 2019

datapythonista commented Dec 29, 2019

HughKelley commented Dec 31, 2019

DOC: Fixes to docstrings, mainly PR09 errors #29530

DOC: Fixes to docstrings, mainly PR09 errors #29530

Conversation

HughKelley commented Nov 10, 2019 • edited Loading

pep8speaks commented Nov 10, 2019 • edited Loading

Comment last updated at 2019-12-31 19:55:57 UTC

jreback commented Nov 11, 2019

HughKelley commented Nov 11, 2019

HughKelley commented Nov 11, 2019

datapythonista commented Nov 11, 2019

datapythonista commented Nov 11, 2019

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

datapythonista commented Nov 12, 2019

jorisvandenbossche commented Nov 19, 2019 • edited Loading

HughKelley commented Nov 19, 2019

jorisvandenbossche commented Nov 19, 2019 • edited Loading

datapythonista commented Nov 19, 2019

HughKelley commented Dec 11, 2019

datapythonista commented Dec 11, 2019

HughKelley commented Dec 11, 2019

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

HughKelley commented Dec 11, 2019

datapythonista commented Dec 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Dec 29, 2019

HughKelley commented Dec 31, 2019

HughKelley commented Nov 10, 2019 •

edited

Loading

pep8speaks commented Nov 10, 2019 •

edited

Loading

jorisvandenbossche commented Nov 19, 2019 •

edited

Loading

jorisvandenbossche commented Nov 19, 2019 •

edited

Loading