CI: add pydocstyle to code_checks #32033

simonjayhawkins · 2020-02-15T22:21:31Z

http://www.pydocstyle.org/en/5.0.2/error_codes.html

This reverts commit b6bd7a5.

jreback · 2020-02-16T16:37:03Z

looks ok to me. @jorisvandenbossche

jorisvandenbossche · 2020-02-16T16:45:34Z

How does this contrast / overlap with the validations we already do with numpydoc/ the validate_docstrings script?

simonjayhawkins · 2020-02-16T17:27:26Z

many differences so will only give the main points below. can go into more fine-grained detail if required.

pydocstyle is a static checker so is not able to check dynamically created docstrings.

validate_docstrings script is only checking public api. pydocstyle is checking internal docstrings too.

pydocstyle and validate_docstrings have some overlapping but also exclusive error codes so having both is beneficial.

pydocstyle has a flake8 plugin so can see errors highlighted in ide so a benefit for contributors.

not added here, but error code 'D417 | Missing argument descriptions in the docstring' would add a lot more than consistent style. It would ensure contributors properly document their functions/methods. A lot of work to enable this globally.

WillAyd · 2020-02-16T18:44:06Z

lgtm. I think we can start with this and refine over time if conflicts with numpydoc arise

requirements-dev.txt

simonjayhawkins · 2020-02-16T19:41:31Z

lgtm. I think we can start with this and refine over time if conflicts with numpydoc arise

Ok. we could enable straight away and see how things pan out. so will take off draft status.

i suppose if issues do arise can always disable. The error codes included here are the simpler ones and it was not a lot of effort to make the changes across the codebase to comply.

we could then look to update the style guide once happy that having on the ci is smooth.

jorisvandenbossche · 2020-02-16T19:43:21Z

error code 'D417 | Missing argument descriptions in the docstring' would add a lot more than consistent style

That's something that numpydoc checks as well?
And actually, as far as I can see, pydocstyle cannot do this for numpy-style docstrings?

jorisvandenbossche · 2020-02-16T19:51:44Z

pandas/core/arrays/datetimes.py

@@ -820,7 +819,7 @@ def tz_convert(self, tz):
        dtype = tz_to_dtype(tz)
        return self._simple_new(self.asi8, dtype=dtype, freq=self.freq)

-    def tz_localize(self, tz, ambiguous="raise", nonexistent="raise"):
+    def tz_localize(self, tz, ambiguous="raise", nonexistent="raise"):  # noqa


Why is the noqa needed here?

another continuation line issue.

Can we rather figure out what is wrong, or disable the indentation check?

probably need a fix upstream

looks like an issue has already been raised for this PyCQA/pydocstyle#437

jorisvandenbossche · 2020-02-16T19:52:24Z

pandas/core/tools/datetimes.py

@@ -538,7 +538,7 @@ def to_datetime(
    infer_datetime_format=False,
    origin="unix",
    cache=True,
-):
+):  # noqa: D207


same question here?

there looks to be a problem with the use of continuation. The next line is flagged as underindented (D207).

It's black that requires this, so I would maybe not include the D207 check then, if it gives conflicts with black.

disabling D207 on one or two functions is better than disabling on all, surely?

see PyCQA/pydocstyle#234

simonjayhawkins · 2020-02-16T20:07:48Z

That's something that numpydoc checks as well?
And actually, as far as I can see, pydocstyle cannot do this for numpy-style docstrings?

can you elaborate?

the checks of the parameters aren't broken down into separate codes for the parameter violations and the checks are not so rigorous.

sample cli output of D417

$ pydocstyle pandas/io/formats/format.py  --select=D417
pandas/io/formats/format.py:968 in public method `to_html`:
        D417: Missing argument descriptions in the docstring (argument(s) buf, encoding are missing descriptions in 'to_html' docstring)
pandas/io/formats/format.py:1111 in public function `format_array`:
        D417: Missing argument descriptions in the docstring (argument(s) decimal, digits, float_format, formatter, justify, na_rep, space, values are missing descriptions in 'format_array' docstring)
pandas/io/formats/format.py:1507 in public function `format_percentiles`:
        D417: Missing argument descriptions in the docstring (argument(s) percentiles are missing descriptions in 'format_percentiles' docstring)
(pandas-dev)

…wkins/pandas into pydocstyle

datapythonista

lgtm

datapythonista · 2020-02-17T08:09:01Z

ci/code_checks.sh

@@ -316,6 +316,11 @@ fi
 ### DOCSTRINGS ###
 if [[ -z "$CHECK" || "$CHECK" == "docstrings" ]]; then

+
+    MSG='Validate docstrings (D201, D202, D204, D207, D208, D209, D213, D300, D409, D411, D412, D414)' ; echo $MSG
+    pydocstyle pandas --select=D201,D202,D204,D207,D208,D209,D213,D300,D409,D411,D412,D414


Would it make sense to have the codes in setup.cfg? I guess we could then call pydocstyle pandas locally and get this validation.

This reverts commit fe25c6b.

WillAyd

lgtm @jorisvandenbossche

jorisvandenbossche · 2020-02-19T20:37:13Z

@simonjayhawkins Thanks for the explanation above #32033 (comment) So I suppose the main reason to use it in addition to numpydoc is that it checks all docstrings?

And actually, as far as I can see, pydocstyle cannot do this for numpy-style docstrings?

can you elaborate?

That was mentioned in one of the issues on numpydoc that talked about using pydocstyle, but it was from a while ago, so probably out of date

If this is going to fail CI in case of violation, we should add this to the contributing guidelines (similar to black and flake8)

Also, should we add it to pre-commit hooks? (can it run on only the changed files? Or how fast it is for the full code base?)

jorisvandenbossche

So looks good, but needs some docs IMO

alimcmaster1 · 2020-02-22T23:16:43Z

Maybe worth a note in the contributing guide?

simonjayhawkins · 2020-02-28T11:02:17Z

I seem incapable of successfully building documentation locally on my Windows machine. closing to clear queue

jorisvandenbossche · 2020-02-28T13:31:48Z

@simonjayhawkins for only adding some prose, there is not necessarily a need to build the docs locally. I can certainly review on technical rst aspects.

simonjayhawkins · 2020-02-28T13:42:02Z

I think the ideal (but not most straightforward approach) is to update https://dev.pandas.io/docs/development/contributing_docstring.html.

I think that we need to avoid confusion between the public API functions/methods/classes that use sphinx to generate documentation for pandas users and docstrings for all other (internal) functions/method/classes that are for the benefit of pandas contributors.

A simpler approach for now could be to add a one-liner in the contributing guide instead.

WillAyd · 2020-03-17T00:12:43Z

Do we have a go forward on this? I still think OK and would be fine with doc updates as a follow on since this isn't drastically changing things.

Though not a strong preference either way - just don't want this to hang in limbo if it doesn't need to

jorisvandenbossche · 2020-03-17T07:20:10Z

If it is going to fail CI, it should be documented in the contributing docs. It doesn't need to belong, just a short mention of it should be fine.

Also:

Also, should we add it to pre-commit hooks? (can it run on only the changed files? Or how fast it is for the full code base?)

simonjayhawkins · 2020-03-17T08:40:05Z

If it is going to fail CI, it should be documented in the contributing docs.

#32393 was intended as precursor to this. Do we have the contributing docs available now? if so, can you provide a link, and i'll reopen #32393.

simonjayhawkins · 2020-03-17T08:41:43Z

Also, should we add it to pre-commit hooks?

i don't have these (may require a bit of effort/knowledge to setup on Windows). so will need others to push the relevant patch for this.

simonjayhawkins · 2020-03-17T08:47:17Z

Though not a strong preference either way - just don't want this to hang in limbo if it doesn't need to

no strong preference either. see #32033 (comment).

simonjayhawkins · 2020-03-17T08:49:44Z

I think we need further discussion before we address #32033 (comment) and then update the guidance. I don't think this PR is that vehicle.

simonjayhawkins · 2020-03-17T12:13:37Z

I think we need further discussion before we address #32033 (comment) and then update the guidance. I don't think this PR is that vehicle.

opened #32773, closing this to clear the queue for now.

CI: add pydocstyle to code_checks

979dcca

simonjayhawkins added CI Continuous Integration Code Style Code style, linting, code_checks labels Feb 15, 2020

simonjayhawkins added 2 commits February 15, 2020 23:22

check fail

b6bd7a5

Revert "check fail"

719e602

This reverts commit b6bd7a5.

WillAyd approved these changes Feb 16, 2020

View reviewed changes

WillAyd requested changes Feb 16, 2020

View reviewed changes

requirements-dev.txt Show resolved Hide resolved

simonjayhawkins marked this pull request as ready for review February 16, 2020 19:41

simonjayhawkins changed the title ~~POC: CI: add pydocstyle to code_checks~~ CI: add pydocstyle to code_checks Feb 16, 2020

jorisvandenbossche reviewed Feb 16, 2020

View reviewed changes

simonjayhawkins added 3 commits February 16, 2020 20:48

Merge remote-tracking branch 'upstream/master' into pydocstyle

48ef670

add TODO for #noqa

39242d2

Merge branches 'pydocstyle' and 'pydocstyle' of github.com:simonjayha…

9fea852

…wkins/pandas into pydocstyle

datapythonista approved these changes Feb 17, 2020

View reviewed changes

simonjayhawkins added 7 commits February 17, 2020 09:16

add pydocstyle to setup.cfg

89baa1c

update code-checks.sh

ed4b764

check failures

fe25c6b

Merge remote-tracking branch 'upstream/master' into pydocstyle

95a72bd

Revert "check failures"

e3ca7f7

This reverts commit fe25c6b.

Merge remote-tracking branch 'upstream/master' into pydocstyle

b269a0a

Merge remote-tracking branch 'upstream/master' into pydocstyle

3de53d0

simonjayhawkins requested a review from WillAyd February 18, 2020 11:09

WillAyd approved these changes Feb 18, 2020

View reviewed changes

jorisvandenbossche requested changes Feb 19, 2020

View reviewed changes

simonjayhawkins closed this Feb 28, 2020

jorisvandenbossche reopened this Feb 28, 2020

simonjayhawkins closed this Mar 17, 2020

simonjayhawkins mentioned this pull request Mar 17, 2020

DISC: Validating internal docstring presence, completeness and style #32773

Open

simonjayhawkins mentioned this pull request Sep 12, 2021

TYP: ExtensionArray.take accept np.ndarray #43418

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: add pydocstyle to code_checks #32033

CI: add pydocstyle to code_checks #32033

simonjayhawkins commented Feb 15, 2020 •

edited

Loading

jreback commented Feb 16, 2020

jorisvandenbossche commented Feb 16, 2020 •

edited

Loading

simonjayhawkins commented Feb 16, 2020

WillAyd commented Feb 16, 2020

simonjayhawkins commented Feb 16, 2020

jorisvandenbossche commented Feb 16, 2020

jorisvandenbossche Feb 16, 2020

simonjayhawkins Feb 16, 2020

jorisvandenbossche Feb 16, 2020

simonjayhawkins Feb 16, 2020

simonjayhawkins Feb 16, 2020

jorisvandenbossche Feb 16, 2020

simonjayhawkins Feb 16, 2020

jorisvandenbossche Feb 16, 2020

simonjayhawkins Feb 16, 2020

simonjayhawkins Feb 16, 2020

simonjayhawkins commented Feb 16, 2020

datapythonista left a comment

datapythonista Feb 17, 2020

WillAyd left a comment

jorisvandenbossche commented Feb 19, 2020

jorisvandenbossche left a comment

alimcmaster1 commented Feb 22, 2020

simonjayhawkins commented Feb 28, 2020

jorisvandenbossche commented Feb 28, 2020

simonjayhawkins commented Feb 28, 2020

WillAyd commented Mar 17, 2020

jorisvandenbossche commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

CI: add pydocstyle to code_checks #32033

CI: add pydocstyle to code_checks #32033

Conversation

simonjayhawkins commented Feb 15, 2020 • edited Loading

jreback commented Feb 16, 2020

jorisvandenbossche commented Feb 16, 2020 • edited Loading

simonjayhawkins commented Feb 16, 2020

WillAyd commented Feb 16, 2020

simonjayhawkins commented Feb 16, 2020

jorisvandenbossche commented Feb 16, 2020

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

simonjayhawkins commented Feb 16, 2020

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

jorisvandenbossche commented Feb 19, 2020

jorisvandenbossche left a comment

Choose a reason for hiding this comment

alimcmaster1 commented Feb 22, 2020

simonjayhawkins commented Feb 28, 2020

jorisvandenbossche commented Feb 28, 2020

simonjayhawkins commented Feb 28, 2020

WillAyd commented Mar 17, 2020

jorisvandenbossche commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Mar 17, 2020

simonjayhawkins commented Feb 15, 2020 •

edited

Loading

jorisvandenbossche commented Feb 16, 2020 •

edited

Loading