unitaryfund / mitiq

Mitiq is an open source toolkit for implementing error mitigation techniques on most current intermediate-scale quantum computers.
https://mitiq.readthedocs.io
GNU General Public License v3.0
363 stars 160 forks source link

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

Closed cosenal closed 1 month ago

cosenal commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

https://github.com/unitaryfund/mitiq/blob/483ec9257595d403cba1cd7c92f95e4a7ba74fd2/pyproject.toml#L6

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"
cosenal commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

Coincidentally https://github.com/unitaryfund/mitiq/pull/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 commented 1 month ago

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 👌