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

DOC: Prepare to add first docstrings checks to the CI #23560

Merged
merged 3 commits into from
Nov 11, 2018
Merged

DOC: Prepare to add first docstrings checks to the CI #23560

merged 3 commits into from
Nov 11, 2018

Conversation

datapythonista
Copy link
Member

@datapythonista datapythonista commented Nov 8, 2018

Fixing some docstring issues, so CI doesn't fail when we add the first validation of docstrings in #22854.

  • closes #
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@datapythonista datapythonista added Docs CI Continuous Integration Code Style Code style, linting, code_checks labels Nov 8, 2018
@pep8speaks
Copy link

Hello @datapythonista! Thanks for submitting the PR.

@datapythonista
Copy link
Member Author

These are the counts of errors of each category. This PR fixes and validates the first two:

Summary contains heading whitespaces	2
Do not import {imported_library}, as it is imported automatically for the examples (numpy as np, pandas as pd)	3
No Yields section found	6
Wrong parameters order. Actual: {actual_params}. Documented: {documented_params}	15
Parameter "{param_name}" type should not finish with "."	19
Private classes ({mentioned_private_classes}) should not be mentioned in public docstrings	32
Summary must start with infinitive verb, not third person (e.g. use "Generate" instead of "Generates")	58
Description should be capitalized for See Also "{reference_name}" reference	61
Parameter "{param_name}" has no type	61
No summary found (a short summary in a single line should be present at the beginning of the docstring)	92
Summary does not start with a capital letter	113
Missing period at end of description for See Also "{reference_name}" reference	164
No Returns section found	179
Examples do not pass tests:\n{doctest_log}	179
Parameter "{param_name}" description should start with a capital letter	188
{reference_name} in `See Also` section does not need `pandas` prefix, use {right_reference} instead.	204
Use only one blank line to separate sections or paragraphs	217
Unknown parameters {unknown_params}	224
Missing description for See Also "{reference_name}" reference	250
flake8 error: {error_code} {error_message}{times_happening}	275
Summary should fit in a single line	304
Parameter "{param_name}" has no description	353
Parameter "{param_name}" type should use "{right_type}" instead of "{wrong_type}"	396
Parameter "{param_name}" description should finish with "."	413
Closing quotes should be placed in the line after the last text in the docstring (do not close the quotes in the same line as the text, or leave a blank line between the last text and the quotes)	413
Parameters {missing_params} not documented	452
Docstring text (summary) should start in the line immediately after the opening quotes (not in the same line, or leaving a blank line in between)	475
Summary does not end with a period	494

I'll create some "good first issues" once this is merged.

CC: @TomAugspurger @jorisvandenbossche

@jorisvandenbossche
Copy link
Member

Cool!
Did you first try leaving an error to see what travis says?

@jorisvandenbossche
Copy link
Member

Ah, actually, Travis is timing out on this ...

Should we maybe make one build just for all linting? (without running the actual test suite, to have a shorter build) Although you also have some work to move linting to Azure?

@datapythonista
Copy link
Member Author

Yes, I'm still thinking what would be the best option, but I think running the code checks in an independent job will make things simpler in the CI files, and moving it to azure will make it easier for users to see the errors.

I think building the docs is quite slow too, but may be makes sense to have a job in azure for both, if it's not too long?

@jorisvandenbossche
Copy link
Member

I think building the docs is quite slow too,

Yes, as long as it is on travis, we need to keep that separate (not add the linting to that), as this can already be quite long.

Depending on how far in the future the "moving linting to azure" is, I would maybe here simply move all linting to a new dedicated travis build.

@datapythonista
Copy link
Member Author

Moving linting to azure is very close in the future: #22854. :)

I just want to think a bit more, if it makes sense to publish the artifacts of a build and save the resources of another conda setup and pandas build, or if that's too much complexity, and we should have an independent build for this.

I'd start by the simplest option and have an independent build, but azure was quite slow last days, and I didn't like adding the extra work. Auto-canceling the superseded builds in azure should be ready in the next hours (#23523 (comment)) so I think it's again the best option.

@codecov
Copy link

codecov bot commented Nov 8, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #23560      +/-   ##
==========================================
- Coverage   92.25%   92.24%   -0.02%     
==========================================
  Files         161      161              
  Lines       51277    51278       +1     
==========================================
- Hits        47305    47299       -6     
- Misses       3972     3979       +7
Flag Coverage Δ
#multiple 90.62% <ø> (-0.02%) ⬇️
#single 42.28% <ø> (-0.04%) ⬇️
Impacted Files Coverage Δ
pandas/core/frame.py 97.03% <ø> (ø) ⬆️
pandas/core/series.py 93.7% <ø> (ø) ⬆️
pandas/core/generic.py 96.81% <ø> (ø) ⬆️
pandas/core/panel.py 97.91% <ø> (ø) ⬆️
pandas/util/testing.py 86.05% <0%> (-0.66%) ⬇️

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 b930303...9047183. Read the comment docs.

@datapythonista datapythonista changed the title WIP: Adding first docstrings checks to the CI DOC: Prepare to add first docstrings checks to the CI Nov 10, 2018
@datapythonista
Copy link
Member Author

@jorisvandenbossche I'm leaving here the fixes to the docstrings, but I removed the validation of docstrings in the CI from this PR. So we avoid time outs with travis. Once this is merged, I'll add the validation to #22854.

@datapythonista
Copy link
Member Author

@gfyoung if you don't mind taking a look at this one. Is similar to the one we just merged, #23600, but fixing other docstring errors.

@datapythonista datapythonista merged commit 2cea659 into pandas-dev:master Nov 11, 2018
thoo added a commit to thoo/pandas that referenced this pull request Nov 11, 2018
…fixed

* upstream/master:
  DOC: Fixes to docstring to add validation to CI (pandas-dev#23560)
  DOC: Remove incorrect periods at the end of parameter types (pandas-dev#23600)
  MAINT: tm.assert_raises_regex --> pytest.raises (pandas-dev#23592)
  DOC: Updating Series.resample and DataFrame.resample docstrings (pandas-dev#23197)
  ENH: Support for partition_cols in to_parquet (pandas-dev#23321)
  TST: Use intp as expected dtype in IntervalIndex indexing tests (pandas-dev#23609)
thoo added a commit to thoo/pandas that referenced this pull request Nov 11, 2018
* upstream/master:
  BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index (pandas-dev#23524)
  BUG: Delegate more of Excel parsing to CSV (pandas-dev#23544)
  API: DataFrame.__getitem__ returns Series for sparse column (pandas-dev#23561)
  CLN: use float64_t consistently instead of double, double_t (pandas-dev#23583)
  DOC: Fix Order of parameters in docstrings (pandas-dev#23611)
  TST: Unskip some Categorical Tests (pandas-dev#23613)
  TST: Fix integer ops comparison test (pandas-dev#23619)
  DOC: Fixes to docstring to add validation to CI (pandas-dev#23560)
  DOC: Remove incorrect periods at the end of parameter types (pandas-dev#23600)
  MAINT: tm.assert_raises_regex --> pytest.raises (pandas-dev#23592)
  DOC: Updating Series.resample and DataFrame.resample docstrings (pandas-dev#23197)
JustinZhengBC pushed a commit to JustinZhengBC/pandas that referenced this pull request Nov 14, 2018
tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
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
CI Continuous Integration Code Style Code style, linting, code_checks Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants