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 linting #7096

Closed
1 of 2 tasks
shadeMe opened this issue Feb 26, 2024 · 4 comments · Fixed by #7463
Closed
1 of 2 tasks

Docstring linting #7096

shadeMe opened this issue Feb 26, 2024 · 4 comments · Fixed by #7463
Assignees
Labels
2.x Related to Haystack v2.0 P1 High priority, add to the next sprint topic:CI type:documentation Improvements on the docs
Milestone

Comments

@shadeMe
Copy link
Collaborator

shadeMe commented Feb 26, 2024

Tasks

@shadeMe shadeMe added type:documentation Improvements on the docs topic:CI 2.x Related to Haystack v2.0 labels Feb 26, 2024
@shadeMe shadeMe added this to the 2.1.0 milestone Feb 26, 2024
@masci masci added the P2 Medium priority, add to the next sprint if no P1 available label Mar 1, 2024
@masci masci added P1 High priority, add to the next sprint and removed P2 Medium priority, add to the next sprint if no P1 available labels Mar 22, 2024
@davidsbatista
Copy link
Contributor

pydocstyle is currently archived and deprecate in favor of ruff: PyCQA/pydocstyle#658

I will look into ruff

@davidsbatista
Copy link
Contributor

ruff supported rules for pydocstyle

https://docs.astral.sh/ruff/rules/#pydocstyle-d

@davidsbatista
Copy link
Contributor

one downside is that every single public functions needs to have a docstring, but in some cases one doesn't want to have that enforced, e.g: small utility functions

this could be avoided with the pylint parameter: docstring-min-length which is still yet to be implemented in ruff astral-sh/ruff#5860

@davidsbatista
Copy link
Contributor

I would like to add the rules below from pydocstyle which ruff supports, and go in line with the big docstring refactoring with did a while ago:

  • "D102", # Missing docstring in public method
  • "D103", # Missing docstring in public function
  • "D205", # 1 blank line required between summary line and description
  • "D417", # undocumented-parameter
  • "D419", # undocumented-returns

but this will force us to add docstrings to every single function, in summary:

  • D103 Missing docstring in public function (3 instances)
  • D102 Missing docstring in public method (158 instances)
  • D205 1 blank line required between summary line and description (182 instances)

Alternatively we could go with pylint, which is less expressive, as far as I understood only has these 3 rules:

  • missing-module-docstring
  • missing-class-docstring
  • missing-function-docstring

but allows to control which functions/methods need a docstring based on the number of lines:

  • docstring-min-length=10
  • missing-function-docstring

using this config would require for now only 17 changes.

My proposal: use pylint as described and keep an eye on astral-sh/ruff#5860, when it's implemented we can swtich to ruff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Related to Haystack v2.0 P1 High priority, add to the next sprint topic:CI type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants