Enable missing argument in docstring lint by mtreinish · Pull Request #15721 · Qiskit/qiskit

mtreinish · 2026-02-23T17:13:35Z

Summary

In the recently merged #15603 we migrated from pylint to use ruff. During the development of that PR the rule group "D" for rules derived from pydocstyle was investigated but the full rule group was overly pedantic and even with disabling the most egregious rules also changed the docs for the worse (at least by autofixing many rules) so that sphinx would not succesfully build after applying the fixes. So during #15603 the changes for this rule group were reverted. However, one of the useful rules from that group D417 which checks that all arguments are documented. This rule found real issues in the documentation and it is worthwhile to enable it. This commit enables the rule in our ruff checks and fixes the issues associated with.

During this process ruff flagged one place in the __call__ method docstring for the TwoQubitControlledUDecomposer class has not documented several arguments on the method. However, these arguments are unused currently. It is apparently trying to match the __call__ signature of the other two qubit decomposers but is not using most of the arguments. Most could be conceivably supported by TwoQubitControlledUDecomposer and it's not clear why they were added. But, for this PR I didn't want to look into adding the support as it was out of scope. For the time being this PR suppresses the ruff rule and adds a TODO comment to start using the unused arguments.

Details and comments

In the recently merged Qiskit#15603 we migrated from pylint to use ruff. During the development of that PR the rule group "D" for rules derived from pydocstyle was investigated but the full rule group was overly pedantic and even with disabling the most egregious rules also changed the docs for the worse (at least by autofixing many rules) so that sphinx would not succesfully build after applying the fixes. So during Qiskit#15603 the changes for this rule group were reverted. However, one of the useful rules from that group D417 which checks that all arguments are documented. This rule found real issues in the documentation and it is worthwhile to enable it. This commit enables the rule in our ruff checks and fixes the issues associated with. During this process ruff flagged one place in the `__call__` method docstring for the `TwoQubitControlledUDecomposer` class has not documented several arguments on the method. However, these arguments are unused currently. It is apparently trying to match the `__call__` signature of the other two qubit decomposers but is not using most of the arguments. Most could be conceivably supported by `TwoQubitControlledUDecomposer` and it's not clear why they were added. But, for this PR I didn't want to look into adding the support as it was out of scope. For the time being this PR suppresses the ruff rule and adds a TODO comment to start using the unused arguments.

qiskit-bot · 2026-02-23T17:13:40Z

One or more of the following people are relevant to this code:

@enavarro51
@Cryoris
@Qiskit/terra-core
@ajavadia
@levbishop
@t-imamichi

jakelishman

There's only one potentially actionable comment - feel free to either respond to it or just queue for merge.

jakelishman · 2026-02-25T02:34:18Z

 def _run_workflow(
    program: Any,
    pass_manager: BasePassManager,
    *,
    initial_property_set: dict[str, object] | None = None,
    **kwargs,
 ) -> Any:
    """Run single program optimization with a pass manager.

    Args:
        program: Arbitrary program to optimize.
        pass_manager: Pass manager with scheduled passes.
+        initial_property_set: An optional dictionary to preseed the
+            property set in the pass manager with.
        **kwargs: Keyword arguments for IR conversion.


This is a private function - does the Ruff rule enforce that all private functions have complete "Args" definitions, or just ones that specify an "Args" section? (I guess/hope the latter.)

It's the latter, it only looks at docstrings with an Args section: https://docs.astral.sh/ruff/rules/undocumented-param/#pydocstyle-d

jakelishman · 2026-02-25T02:37:13Z

-    Parameters:
+    Args:
        circuit: Input quantum circuit.


I don't mind standardising these, but Napoleon documents "Args" as an alias to "Parameters", so they should be all internally normalised into the same section heading anyway.

Yeah, it was a bit annoying, but the rule checker only looks for Args it doesn't know about Parameters. I assume it's because the rule documents itself as only working with Google style docs.

jakelishman · 2026-02-25T02:39:21Z

            default `left` will be used.
        idle_wires (bool): Include idle wires. Default is True.
        wire_order (list): A list of ints that modifies the order of the bits.
+        wire_map (dict): The wire map


lol

(I don't care about improving this necessarily - it's just an internal helper and clear from the context it's called in.)

Yeah I had the same attitude, I didn't feel the need to figure out the dictionary format and write a detailed description of it just for a private docstring

Cryoris

This also need rebasing on main but otherwise LGTM 🙂

Cryoris

LGTM!

* Enable missing argument in docstring lint In the recently merged Qiskit#15603 we migrated from pylint to use ruff. During the development of that PR the rule group "D" for rules derived from pydocstyle was investigated but the full rule group was overly pedantic and even with disabling the most egregious rules also changed the docs for the worse (at least by autofixing many rules) so that sphinx would not succesfully build after applying the fixes. So during Qiskit#15603 the changes for this rule group were reverted. However, one of the useful rules from that group D417 which checks that all arguments are documented. This rule found real issues in the documentation and it is worthwhile to enable it. This commit enables the rule in our ruff checks and fixes the issues associated with. During this process ruff flagged one place in the `__call__` method docstring for the `TwoQubitControlledUDecomposer` class has not documented several arguments on the method. However, these arguments are unused currently. It is apparently trying to match the `__call__` signature of the other two qubit decomposers but is not using most of the arguments. Most could be conceivably supported by `TwoQubitControlledUDecomposer` and it's not clear why they were added. But, for this PR I didn't want to look into adding the support as it was out of scope. For the time being this PR suppresses the ruff rule and adds a TODO comment to start using the unused arguments. * Update filename docstring * Revise wording on state visualization filename arg

mtreinish requested review from a team as code owners February 23, 2026 17:13

mtreinish added type: qa Issues and PRs that relate to testing and code quality documentation Something is not clear or an error documentation Changelog: None Do not include in the GitHub Release changelog. labels Feb 23, 2026

Merge remote-tracking branch 'origin/main' into ruff-D417

b225abc

jakelishman previously approved these changes Feb 25, 2026

View reviewed changes

mtreinish added 2 commits March 2, 2026 08:16

Merge remote-tracking branch 'origin/main' into ruff-D417

c6d2948

Update filename docstring

c035b03

mtreinish dismissed jakelishman’s stale review via c035b03 March 2, 2026 13:21

mtreinish requested a review from jakelishman March 2, 2026 15:07

Cryoris reviewed Mar 27, 2026

View reviewed changes

Comment thread qiskit/synthesis/two_qubit/two_qubit_decompose.py

Comment thread qiskit/visualization/state_visualization.py Outdated

mtreinish added 2 commits March 27, 2026 07:04

Merge remote-tracking branch 'origin/main' into ruff-D417

3b8e1c5

Revise wording on state visualization filename arg

e614f4f

mtreinish requested a review from Cryoris March 27, 2026 11:08

Cryoris approved these changes Mar 27, 2026

View reviewed changes

Cryoris added this pull request to the merge queue Mar 27, 2026

Merged via the queue into Qiskit:main with commit 5fce32f Mar 27, 2026
24 of 25 checks passed

mtreinish deleted the ruff-D417 branch March 28, 2026 10:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable missing argument in docstring lint#15721

Enable missing argument in docstring lint#15721
Cryoris merged 6 commits intoQiskit:mainfrom
mtreinish:ruff-D417

mtreinish commented Feb 23, 2026

Uh oh!

qiskit-bot commented Feb 23, 2026

Uh oh!

jakelishman left a comment

Uh oh!

jakelishman Feb 25, 2026

Uh oh!

mtreinish Feb 25, 2026

Uh oh!

jakelishman Feb 25, 2026

Uh oh!

mtreinish Feb 25, 2026

Uh oh!

jakelishman Feb 25, 2026

Uh oh!

mtreinish Feb 25, 2026

Uh oh!

Uh oh!

Cryoris left a comment

Uh oh!

Uh oh!

Uh oh!

Cryoris left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

mtreinish commented Feb 23, 2026

Summary

Details and comments

Uh oh!

qiskit-bot commented Feb 23, 2026

Uh oh!

jakelishman left a comment

Choose a reason for hiding this comment

Uh oh!

jakelishman Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

mtreinish Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

jakelishman Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

mtreinish Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

jakelishman Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

mtreinish Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Cryoris left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Cryoris left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants