Skip to content

Conversation

@snehankekre
Copy link
Contributor

πŸ“š Context

#5776 adds a pydocstyle pre-commit hook to ensure any modified or new .py files in (lib\/streamlit\/)(?!vendor)(?!watcher) follow certain stylistic rules. For the full set of rules, see #5776.

That PR however didn't answer the question: "How do we ensure existing docstrings are validated ?".

  • What kind of change does this PR introduce?

    • Bugfix
    • Feature
    • Refactoring
    • Other, please describe: Doc improvement request

🧠 Description of Changes

This PR is a small effort towards answering that question. Rather than fix all the docstring violations, this PR fixes categories of violations (aka Error Codes) that occur ≀ 10 times.

The following errors for the entire codebase have been fixed by this PR: D200, D202, D209, D301, D404, D406, D409.

@snehankekre snehankekre added the security-assessment-completed Security assessment has been completed for PR label Nov 28, 2022
@lukasmasuch lukasmasuch requested a review from vdonato December 1, 2022 19:17
Copy link
Collaborator

@vdonato vdonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- just had some small suggested wording changes + conversion of things that probably shouldn't be docstrings into normal comments

@snehankekre
Copy link
Contributor Author

Thanks, Vincent! I've committed your changes with 7261cf3.

@vdonato vdonato merged commit 05225d9 into streamlit:develop Dec 5, 2022
tconkling added a commit that referenced this pull request Dec 6, 2022
* develop:
  Fix multipage apps e2e test describe block (#5810)
  Replace closeConnection with shutdownRuntime and disconnectWebsocket (#5809)
  Fix infrequently occurring (≀ 10) pydocstyle error codes (#5777)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants