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

Docstring improvements & enable lints #4161

Merged
merged 10 commits into from
Jul 27, 2022
Merged

Conversation

stinodego
Copy link
Contributor

@stinodego stinodego commented Jul 26, 2022

Builds upon #4155

Fixes #3975

Changes:

  • Enabled flake8-docstrings in the CI. You will now get slapped if you forget to add a blank line before the end of your docstring 😄 Not all lints are enabled yet (see notes below).
  • Fixed all docstrings to be compatible with the linter

Notes:

  • Lints for missing docstrings are disabled. We should add these, but doing so will require quite an effort.
  • Docstring conventions require a single line description, possibly followed by multiple lines of elaboration. Lints for this are disabled for now: We do not always have this distinction, and it is not always easy to succinctly describe a function in a single line. Requires some effort to implement.

If it turns out there are some controversial lints that bug you while documenting your code, let's discuss and we can adjust the linter as necessary. This is just a first attempt at enforcing some uniformity!

@github-actions github-actions bot added the python Related to Python Polars label Jul 26, 2022
@ritchie46
Copy link
Member

Thabks @stinodego. Can you do a rebase? Then I merge it in.

@thatlittleboy
Copy link
Contributor

@stinodego thanks for working on this! good to know there is a easy solution to #3975. please do add note about closing #3975 if this PR covers what I was raising in that issue (seems like it does to me)

@stinodego
Copy link
Contributor Author

Done! Thanks guys.

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2022

Codecov Report

Merging #4161 (8fc5e86) into master (8fc5e86) will not change coverage.
The diff coverage is n/a.

❗ Current head 8fc5e86 differs from pull request most recent head fe04038. Consider uploading reports for the commit fe04038 to get more accurate results

@@           Coverage Diff           @@
##           master    #4161   +/-   ##
=======================================
  Coverage   78.75%   78.75%           
=======================================
  Files         457      457           
  Lines       75728    75728           
=======================================
  Hits        59637    59637           
  Misses      16091    16091           

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 8fc5e86...fe04038. Read the comment docs.

@ritchie46 ritchie46 merged commit 8f07335 into pola-rs:master Jul 27, 2022
@stinodego stinodego deleted the docstrings-2 branch August 30, 2022 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Standardization of reST section markups via CI
4 participants