Skip to content

Fix the docstring of CommutationChecker.commute_nodes#15991

Open
Cryoris wants to merge 2 commits intoQiskit:mainfrom
Cryoris:cc/fix-docstring
Open

Fix the docstring of CommutationChecker.commute_nodes#15991
Cryoris wants to merge 2 commits intoQiskit:mainfrom
Cryoris:cc/fix-docstring

Conversation

@Cryoris
Copy link
Copy Markdown
Collaborator

@Cryoris Cryoris commented Apr 10, 2026

I came across this during reviewing Matt's cache PR: With the partial type hints the API docs only showed the last 2 parameters instead of all 4.

image

Also added some more explanations.

AI/LLM disclosure

  • I didn't use LLM tooling, or only used it privately.
  • I used the following tool to help write this PR description:
  • I used the following tool to generate or modify code:

Cryoris added 2 commits April 10, 2026 15:51
With the partial type hints the API docs only showed the last 2 parameters instead of all 4. also added some more explanations.
@Cryoris Cryoris requested a review from a team as a code owner April 10, 2026 13:54
@Cryoris Cryoris added documentation Something is not clear or an error documentation Changelog: None Do not include in the GitHub Release changelog. labels Apr 10, 2026
@qiskit-bot
Copy link
Copy Markdown
Collaborator

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

  • @Qiskit/terra-core

approximation_degree: float = 1.0,
) -> bool:
"""Checks if two DAGOpNodes commute."""
"""Checks if two :class:`.DAGOpNode`\ s commute.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Either

Suggested change
"""Checks if two :class:`.DAGOpNode`\ s commute.
"""Checks if two :class:`.DAGOpNode`\\ s commute.

(bad Python escape character) or

Suggested change
"""Checks if two :class:`.DAGOpNode`\ s commute.
"""Checks if two :class:`.DAGOpNode` objects commute.

The docs team usually prefer the second because it's easier to translate or to read for less-fluent English speakers.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm fairly sure pylint used to complain about this, but it looks like Ruff doesn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: None Do not include in the GitHub Release changelog. documentation Something is not clear or an error documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants