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 string of some function parameters in API doc missing #2472

Closed
cosenal opened this issue Aug 17, 2024 · 8 comments
Closed

Doc string of some function parameters in API doc missing #2472

cosenal opened this issue Aug 17, 2024 · 8 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation good first issue Good for newcomers infrastructure For issues related to building, packaging, and continuous integration. non-quantum Requires no knowledge of quantum physics to fix / work on.

Comments

@cosenal
Copy link
Contributor

cosenal commented Aug 17, 2024

In the API doc, the parameters of some of the functions are missing the docstring after the docs are build, despite it's there in the source code, e.g., https://mitiq.readthedocs.io/en/stable/apidoc.html#module-mitiq.observable.pauli

@cosenal cosenal added bug Something isn't working documentation Improvements or additions to documentation infrastructure For issues related to building, packaging, and continuous integration. labels Aug 17, 2024
@cosenal cosenal changed the title Parameters of some function in API doc missing doc string Doc string of some function parameters in API doc missing Aug 18, 2024
@natestemen natestemen added the good first issue Good for newcomers label Aug 19, 2024
@cosenal
Copy link
Contributor Author

cosenal commented Aug 23, 2024

As observed by @purva-thakre at the Mitiq Community call, it may be as simple as having a new line between the name of the parameter and the description of the parameter.

@purva-thakre
Copy link
Collaborator

purva-thakre commented Aug 23, 2024

On the issue of catching docstring formatting issues, we could enable pydocstyle in ruff to catch errors similar to what's mentioned in the issue description and in #2422

https://docs.astral.sh/ruff/settings/#lintpydocstyle

[tool.ruff.lint]

Edit: If I edit the toml file as shown below, the errors in the description are indeed caught by pydocstyle. If we decide to go down this path, we will have to pick and choose the rules to enforce. Right now, 1277 errors are flagged. 😦

[tool.ruff.lint]
select = [
    # pycodestyle
    "E",
    # pyflakes
    "F",
    # isort
    "I",
    # pydocstyle
    "D",
]

[tool.ruff.lint.pydocstyle]
convention = "google"

@purva-thakre purva-thakre added the non-quantum Requires no knowledge of quantum physics to fix / work on. label Aug 26, 2024
@cosenal
Copy link
Contributor Author

cosenal commented Oct 1, 2024

@purva-thakre Sounds good.
For the assignee, if we go with using the linter to catch those errors, we want the configuration of the linter to be so that it fixes both #2472 and #2422.

@walkingalchemy
Copy link

Does it feel like enforcing a standardized coding style via linter is something the project needs? It is a somewhat blunt solution to the problems in the referenced issues.

If it is a desired path it sounds like there may be some consensus needed for what rules to implement. Can that type of decision be made quickly in this group?

@cosenal
Copy link
Contributor Author

cosenal commented Oct 4, 2024

@walkingalchemy 👋 Note we are proposing specifically linting of the docstring, as oppposed to a general coding style, as a solution to the problems in the referenced issues. If you have a better solution in mind, feel free to suggest it.

what rules to implement

We should start with the minimum set of new rules that fixes this issue along with #2422.

@walkingalchemy
Copy link

Sounds good @cosenal 👋

I am willing to investigate if you'd like to assign this to me. I'm away from my desk for a few more days but I can pick it up next week.

@cosenal
Copy link
Contributor Author

cosenal commented Oct 4, 2024

Coincidentally #2525 fixes the issue for the link reported in the original post of this issue, see same link but on latest build: https://mitiq.readthedocs.io/en/latest/apidoc.html#module-mitiq.observable.pauli
Action on me to figure out if the issue can still happen somewhere else, outside of constructors.

@cosenal
Copy link
Contributor Author

cosenal commented Oct 8, 2024

Closing this as I couldn't reproduce it after #2525. Even after artificially adding a new line between the argument name and description doesn't break it 👌

@cosenal cosenal closed this as completed Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation good first issue Good for newcomers infrastructure For issues related to building, packaging, and continuous integration. non-quantum Requires no knowledge of quantum physics to fix / work on.
Projects
None yet
Development

No branches or pull requests

4 participants