Skip to content

validate_docstrings improvements #25122

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

Conversation

kynan
Copy link
Contributor

@kynan kynan commented Feb 3, 2019

Some minor improvments to validate_docstrings:

  • outputs warning/error codes in the list of validations
  • add --list-errors flag to list error codes / descriptions

@kynan kynan changed the title validate_docstrings outputs warning/error codes validate_docstrings improvements Feb 3, 2019
@WillAyd
Copy link
Member

WillAyd commented Feb 3, 2019

Thanks for the PR.

  • Is this closing an open issue that has been discussed?
  • Can you add tests?

@WillAyd WillAyd added the Docs label Feb 3, 2019
@codecov
Copy link

codecov bot commented Feb 3, 2019

Codecov Report

Merging #25122 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25122      +/-   ##
==========================================
+ Coverage   92.37%   92.37%   +<.01%     
==========================================
  Files         166      166              
  Lines       52415    52415              
==========================================
+ Hits        48418    48419       +1     
+ Misses       3997     3996       -1
Flag Coverage Δ
#multiple 90.79% <ø> (ø) ⬆️
#single 42.86% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/frame.py 96.82% <0%> (ø) ⬆️
pandas/core/arrays/categorical.py 95.97% <0%> (ø) ⬆️
pandas/core/window.py 96.4% <0%> (ø) ⬆️
pandas/util/testing.py 88.13% <0%> (+0.09%) ⬆️

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 a814ea4...7bdefdb. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Feb 3, 2019

Codecov Report

Merging #25122 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25122      +/-   ##
==========================================
+ Coverage   92.37%   92.37%   +<.01%     
==========================================
  Files         166      166              
  Lines       52415    52415              
==========================================
+ Hits        48418    48419       +1     
+ Misses       3997     3996       -1
Flag Coverage Δ
#multiple 90.79% <ø> (ø) ⬆️
#single 42.86% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/frame.py 96.82% <0%> (ø) ⬆️
pandas/core/arrays/categorical.py 95.97% <0%> (ø) ⬆️
pandas/core/window.py 96.4% <0%> (ø) ⬆️
pandas/util/testing.py 88.13% <0%> (+0.09%) ⬆️

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 a814ea4...7bdefdb. Read the comment docs.

@TomAugspurger TomAugspurger added this to the 0.25.0 milestone Feb 3, 2019
@datapythonista
Copy link
Member

I would avoid all these changes based on personal preference. It just adds noise to keep changing tabs by spaces in the output, or adding line breaks to function calls. The code is respecting PEP8, and if every person reformats based on his personal preference, we'll change those at every PR.

Can you revert those, leave the relevant part of the PR, and fix the problems in the CI please?

@WillAyd
Copy link
Member

WillAyd commented Feb 12, 2019

Closing for now due to failures and comments above. If you want to pursue further please open an issue first for discussion

@WillAyd WillAyd closed this Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants