feat: add qbraid_qir.qiskit module for Qiskit to QIR conversion#271
feat: add qbraid_qir.qiskit module for Qiskit to QIR conversion#271eidrynbot wants to merge 4 commits intoqBraid:mainfrom
Conversation
Port of microsoft/qiskit-qir (MIT License) with the following updates: - Compatible with Qiskit 2.x (new QuantumCircuit API) - Uses qiskit_to_qir() interface to match cirq_to_qir/qasm3_to_qir - Supports pyqir 0.10-0.12+ (typed and opaque pointers) - Follows qbraid-qir conventions and code style New files: - qbraid_qir/qiskit/ module with convert, visitor, elements, exceptions - tests/qiskit_qir/ with 44 unit tests - NOTICE.md for MIT license attribution - docs/api/qbraid_qir.qiskit.rst Updated files: - pyproject.toml: added qiskit extra - tox.ini: added qiskit to test environments - README.md: added qiskit usage example - docs/index.rst: added qiskit to API reference - docs/conf.py: added qiskit to autodoc_mock_imports
Follow the pattern from qBraid SDK (azure/result_builder.py): - Keep Apache 2.0 as the primary license - Add pylint comments and attribution block referencing microsoft/qiskit-qir - Reference NOTICE.md for full MIT license text
|
CI workflows may need approval since this is from a fork. I've updated the license headers to follow the pattern from the qBraid SDK (
All local tests pass (225 passed, 5 skipped). Docs build successfully. |
WalkthroughAdds a new qbraid_qir.qiskit subpackage implementing Qiskit→QIR conversion (qiskit_to_qir), visitor-based translation, Qiskit-specific elements and errors, tests, docs, packaging and CI updates, and attribution NOTICE for the ported microsoft/qiskit-qir code. Changes
Sequence DiagramsequenceDiagram
actor User
participant Circuit as Qiskit<br/>QuantumCircuit
participant Convert as qiskit_to_qir<br/>Converter
participant PyQIR as PyQIR<br/>Module
participant QModule as QiskitModule<br/>Wrapper
participant Visitor as BasicQiskitVisitor<br/>Transformer
User->>Convert: qiskit_to_qir(circuit, name?, **kwargs)
Convert->>Convert: validate type & non-empty
Convert->>PyQIR: create Context & Module
Convert->>QModule: QiskitModule.from_circuit(circuit, module)
QModule->>QModule: parse registers & instructions -> elements
Convert->>Visitor: instantiate with kwargs
Convert->>Visitor: visit_qiskit_module(qiskit_module)
Visitor->>Visitor: init entrypoint, builder, runtime (optional)
Visitor->>Visitor: visit_register(...) for each register
Visitor->>Visitor: visit_instruction(... ) for each instruction
Visitor->>Visitor: record_output() (optional) and finalize()
Visitor->>PyQIR: emit QIR bodies & entrypoint attributes
Convert->>PyQIR: verify module
Convert-->>User: return validated PyQIR Module
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 19: Update the CHANGELOG entry for the qbraid_qir.qiskit addition to
reflect the actual function signature of qiskit_to_qir by noting that the name
parameter is optional (defaults to None) rather than required; locate the entry
referencing qiskit_to_qir(circuit, name, **kwargs) and change the description to
show qiskit_to_qir(circuit, name=None, **kwargs) or explicitly state "name is
optional (defaults to None)" so the documentation matches the implementation.
In `@docs/api/qbraid_qir.qiskit.rst`:
- Around line 6-8: The automodule directives (e.g., the one for
qbraid_qir.qiskit) are missing the :members: option so module members
(classes/functions) are not included; update each API rst
(qbraid_qir.qiskit.rst, qbraid_qir.cirq.rst, qbraid_qir.qasm3.rst,
qbraid_qir.squin.rst) to add the :members: option alongside :undoc-members: and
:show-inheritance: in the automodule directive to ensure all module members are
documented.
In `@NOTICE.md`:
- Around line 33-37: NOTICE.md currently does not end with exactly one trailing
newline; update the file to ensure it finishes with a single newline character
(no extra blank lines or missing newline) so it satisfies markdownlint
MD047—open NOTICE.md, remove any extra trailing blank lines if present, and add
exactly one '\n' at EOF.
- Around line 1-3: Update the header and phrasing to use hyphenated
“Third-Party” consistently: change the heading text "Third Party Notices" to
"Third-Party Notices" and change "third party sources" to "third-party sources"
so both the title and body use the hyphenated form.
In `@pyproject.toml`:
- Line 43: Update the Qiskit extra dependency specification in pyproject.toml
(the line currently reading qiskit = ["qiskit>=2.0"]) to constrain to the 2.x
series so future major releases won’t be pulled in; replace the loose ">=2.0"
spec with a 2.x‑bounded spec (e.g., >=2.0,<3.0) to pin the major version while
allowing patch/minor updates.
In `@qbraid_qir/__init__.py`:
- Line 57: _lazy currently maps backends to plain strings which are iterated as
character sequences by __dir__ and __getattr__; change the values in _lazy to
sequences (e.g., tuples or lists) of object/module names (e.g., ("cirq_to_qir",)
not "cirq_to_qir") so downstream logic treats each entry as a single name.
Update the _lazy definition and ensure any code that iterates _lazy (notably
__dir__ and __getattr__) expects sequences of names rather than characters; keep
the same identifiers "cirq", "qasm3", "qiskit" as keys and use singleton
tuples/lists as their values.
In `@qbraid_qir/qiskit/__init__.py`:
- Line 59: The __all__ export list is unsorted which triggers lint churn; in
qiskit/__init__.py, sort the names in the __all__ list alphabetically (e.g.,
"BasicQiskitVisitor", "QiskitConversionError", "QiskitModule", "qiskit_to_qir")
so the exported symbols BasicQiskitVisitor, QiskitConversionError, QiskitModule
and qiskit_to_qir appear in lexicographic order.
In `@qbraid_qir/qiskit/convert.py`:
- Line 36: The public entry function qiskit_to_qir currently uses an untyped
**kwargs; change its signature to document and type the visitor/visitor-related
kwargs (e.g., add a typed keyword-only parameter or type the var-kwargs). For
example, import typing (Any, Optional, Mapping) and replace **kwargs with a
typed parameter such as *, visitors: Optional[Mapping[str, Any]] = None (or keep
**kwargs: Any if you must accept arbitrary keys) and update all internal uses of
kwargs inside qiskit_to_qir to reference the new name (visitors or
visitor_kwargs) so callers and static checkers get proper hints.
In `@qbraid_qir/qiskit/elements.py`:
- Around line 38-49: The base class method
_QuantumCircuitElement.from_element_list creates instances via cls(elem), which
is incompatible with _Instruction.__init__(instruction, qargs, cargs); update
the code so _Instruction explicitly overrides from_element_list (or defines a
classmethod) that either raises NotImplementedError to prevent misuse or
implements the correct construction used in from_circuit, and ensure you
reference _Instruction.__init__ and the existing from_circuit logic when
implementing the override so callers won't attempt to use the base
implementation.
- Around line 97-113: The __init__ method in qbraid_qir/qiskit/elements.py
currently takes eight parameters (circuit, name, module, num_qubits, num_clbits,
reg_sizes, elements) which triggers pylint's too-many-arguments error; fix this
by either 1) adding a local suppression for this constructor (add a pylint
disable comment for too-many-arguments on the __init__ method) if the signature
must remain stable, or 2) refactoring the signature to accept a single
config/dataclass object (e.g., ElementsConfig) that contains circuit, name,
module, num_qubits, num_clbits, reg_sizes, elements and then update the
constructor body that assigns to self._circuit, self._name, self._module,
self._elements, self._num_qubits, self._num_clbits, and self.reg_sizes to pull
values from that config; choose one approach and apply consistently to the
__init__ function to resolve the lint failure.
In `@qbraid_qir/qiskit/visitor.py`:
- Around line 238-323: The visit_instruction method has too many branches and
statements; refactor by introducing gate-dispatch tables (e.g.,
_SINGLE_QUBIT_GATES, _MULTI_QUBIT_GATES, _ROTATION_GATES) that map op_name
strings to qis functions, then replace the long if/elif chain in
visit_instruction with lookups that call the mapped function (passing
self._builder and either *qubits or *instruction.params, *qubits as
appropriate), while keeping special-case handling for "measure"/"m"/"mz",
"barrier", "id", NOOP_INSTRUCTIONS, and composite instructions
(instruction.definition) unchanged (use visit_instruction,
_process_composite_instruction, and qis.* symbols to locate code to modify).
- Line 185: The debug call uses an unnecessary list comprehension; change the
logger.debug invocation that currently formats "Added labels for qubits %s" with
[bit for bit in register] to instead use list(register) so the expression is
clearer and avoids creating an extra comprehension; locate the logger.debug call
in the visitor (the line that logs "Added labels for qubits %s" and uses
register) and replace the comprehension with list(register).
- Around line 263-265: The loop that pairs qubits and results uses zip(qubits,
results) and can silently drop mismatched items; change it to use zip(qubits,
results, strict=True) so mismatched lengths raise an error, and keep the
existing body that sets self._measured_qubits[pointer_id(qubit)] = True and
calls qis.mz(self._builder, qubit, result) to preserve behavior; update the loop
in visitor.py where pointer_id, self._measured_qubits, qis.mz, and self._builder
are referenced.
- Around line 31-32: The imports use module-qualified import style causing a
pylint violation; change the two import statements in qbraid_qir.qiskit.visitor
(currently "import pyqir.qis as qis" and "import pyqir.rt as rt") to the package
import form "from pyqir import qis" and "from pyqir import rt" so the module
symbols qis and rt are imported via the package namespace and satisfy the
linter.
- Around line 86-112: The BasicQiskitVisitor class currently triggers pylint's
too-many-instance-attributes (10/7); to silence this CI error, add a local
pylint pragma that disables too-many-instance-attributes for this class: place a
"# pylint: disable=too-many-instance-attributes" comment immediately before the
class BasicQiskitVisitor declaration (so it covers __init__ and attributes like
_module, _qiskit_module, _builder, _entry_point, _qubit_labels, _clbit_labels,
_measured_qubits, _initialize_runtime, _record_output, _emit_barrier_calls) and
then re-enable the check with "# pylint: enable=too-many-instance-attributes"
immediately after the class definition (or at the end of the file) to limit the
suppression scope.
In `@tests/qiskit_qir/conftest.py`:
- Line 24: Replace the unnecessary empty-call fixture decorators: change every
occurrence of the decorator `@pytest.fixture`() to the bare `@pytest.fixture` for
each fixture in conftest.py (all 13 fixtures that currently use
`@pytest.fixture`() — e.g., the decorators on the fixtures at the noted
locations), so the fixtures use the bare decorator form and satisfy PT001.
In `@tests/qiskit_qir/test_basic_gates.py`:
- Around line 104-107: The current assertion is too permissive because "double"
can appear elsewhere; instead, locate the function IR for
"__quantum__qis__{gate_name}__body" (use the existing ir = str(module) and the
f"__quantum__qis__{gate_name}__body" check), then assert that a precise angle
literal appears in that function body (e.g., look inside the function substring
and require a specific LLVM constant/token such as "double 0.5" or the exact
encoding you expect like "0.5" or "5.0" within that function body) rather than
asserting "double" anywhere in the whole IR. Ensure you reference ir, module and
gate_name when extracting the function substring and replace the broad assertion
with this targeted check.
- Line 57: Replace comma-separated string argnames in the
pytest.mark.parametrize decorators with a tuple of strings to satisfy PT006:
change occurrences like pytest.mark.parametrize("gate_name,qir_name",
SINGLE_QUBIT_GATES.items()) to pytest.mark.parametrize(("gate_name",
"qir_name"), SINGLE_QUBIT_GATES.items()) (also apply the same tuple-style fix
for the other parametrize uses at the same file around the other decorators on
lines where parametrize is used).
In `@tests/qiskit_qir/test_qiskit_to_qir.py`:
- Line 191: Replace the chained or equality in the test assertion by using a
membership check: update the assertion that currently compares module.name ==
bell_circuit.name or module.name == "main" to assert that module.name is in a
collection containing bell_circuit.name and "main" (referencing module.name and
bell_circuit.name in the test_qiskit_to_qir.py assertion).
- Line 167: Update the pytest.raises call in test_qiskit_to_qir.py that
currently uses match="must be of type qiskit.QuantumCircuit" to use a raw,
escaped regex so the dots are treated literally; change the match argument to a
raw string with escaped dots (e.g., r"must be of type qiskit\.QuantumCircuit")
in the pytest.raises(...) invocation to prevent unintended matches for
any-character dots.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f043602-6154-4b38-9018-e50948ff1ef1
📒 Files selected for processing (18)
CHANGELOG.mdNOTICE.mdREADME.mddocs/api/qbraid_qir.qiskit.rstdocs/conf.pydocs/index.rstpyproject.tomlqbraid_qir/__init__.pyqbraid_qir/qiskit/__init__.pyqbraid_qir/qiskit/convert.pyqbraid_qir/qiskit/elements.pyqbraid_qir/qiskit/exceptions.pyqbraid_qir/qiskit/visitor.pytests/qiskit_qir/__init__.pytests/qiskit_qir/conftest.pytests/qiskit_qir/test_basic_gates.pytests/qiskit_qir/test_qiskit_to_qir.pytox.ini
|
|
||
| ### ➕ New Features | ||
|
|
||
| - Added `qbraid_qir.qiskit` module for Qiskit to QIR conversion, ported from the archived [microsoft/qiskit-qir](https://github.com/microsoft/qiskit-qir) repository (MIT License). The module has been updated for compatibility with Qiskit 2.x and follows qbraid-qir conventions. Main entry point is `qiskit_to_qir(circuit, name, **kwargs)` which converts a Qiskit `QuantumCircuit` to a PyQIR `Module`. ([#272](https://github.com/qBraid/qbraid-qir/issues/272), [#271](https://github.com/qBraid/qbraid-qir/pull/271)) |
There was a problem hiding this comment.
Correct the documented function signature for qiskit_to_qir.
The changelog currently implies name is required, but implementation uses name=None (optional). Please align docs to avoid API confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 19, Update the CHANGELOG entry for the
qbraid_qir.qiskit addition to reflect the actual function signature of
qiskit_to_qir by noting that the name parameter is optional (defaults to None)
rather than required; locate the entry referencing qiskit_to_qir(circuit, name,
**kwargs) and change the description to show qiskit_to_qir(circuit, name=None,
**kwargs) or explicitly state "name is optional (defaults to None)" so the
documentation matches the implementation.
| .. automodule:: qbraid_qir.qiskit | ||
| :undoc-members: | ||
| :show-inheritance: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check automodule directive patterns in API docs
fd --glob '*.rst' docs/api | head -20Repository: qBraid/qbraid-qir
Length of output: 204
🏁 Script executed:
#!/bin/bash
# Examine the specific file and surrounding context
if [ -f "docs/api/qbraid_qir.qiskit.rst" ]; then
cat -n "docs/api/qbraid_qir.qiskit.rst"
else
echo "File not found, searching for qiskit.rst files:"
find . -name "*qiskit.rst" -type f 2>/dev/null
fiRepository: qBraid/qbraid-qir
Length of output: 239
🏁 Script executed:
#!/bin/bash
# Check all automodule directives in docs/api to see the pattern
fd --glob '*.rst' docs/api | xargs grep -l "automodule" | while read file; do
echo "=== $file ==="
grep -A 3 "automodule" "$file"
doneRepository: qBraid/qbraid-qir
Length of output: 567
🏁 Script executed:
#!/bin/bash
# Check Sphinx configuration for automodule settings
find docs -name "conf.py" -o -name "conf.rst" | head -5Repository: qBraid/qbraid-qir
Length of output: 73
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about why members aren't included
rg -B2 -A2 "undoc-members|:members:" docs/ --type rstRepository: qBraid/qbraid-qir
Length of output: 849
🌐 Web query:
Sphinx automodule :members: vs :undoc-members: directive difference documentation
💡 Result:
In Sphinx autodoc (e.g., .. automodule:: yourpkg.yourmod):
-
:members:tells Sphinx to include the module’s members (classes, functions, variables, etc.) in the generated docs. Without it, you generally get just the module-level docstring. You can also pass a comma-separated list to document only specific members. [1] -
:undoc-members:tells Sphinx to also include members that have no docstring/doc-comment (i.e., “undocumented” members). This is typically used in addition to:members:to get all members, not just the documented ones. [1]
Example:
.. automodule:: noodles
:members:
:undoc-members:This includes members with docstrings (because of :members:) and those without docstrings (because of :undoc-members:). [1]
Source: Sphinx autodoc options documentation. [1]
[1] https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html
🏁 Script executed:
#!/bin/bash
# Check the Sphinx configuration for autodoc settings
cat -n docs/conf.py | head -100Repository: qBraid/qbraid-qir
Length of output: 4288
🏁 Script executed:
#!/bin/bash
# Look for autodoc_default_options or any special autodoc configuration
rg -n "autodoc|automodule" docs/conf.pyRepository: qBraid/qbraid-qir
Length of output: 414
Add :members: to include actual module members in API documentation.
The :undoc-members: option requires :members: to function properly in Sphinx. Without :members:, the automodule directive produces only module-level content and omits all classes, functions, and other members.
This pattern affects all module API documentation files (qbraid_qir.cirq.rst, qbraid_qir.qasm3.rst, qbraid_qir.qiskit.rst, and qbraid_qir.squin.rst). Add :members: to each to ensure complete API documentation.
Proposed fix
.. automodule:: qbraid_qir.qiskit
+ :members:
:undoc-members:
:show-inheritance:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .. automodule:: qbraid_qir.qiskit | |
| :undoc-members: | |
| :show-inheritance: | |
| .. automodule:: qbraid_qir.qiskit | |
| :members: | |
| :undoc-members: | |
| :show-inheritance: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/api/qbraid_qir.qiskit.rst` around lines 6 - 8, The automodule directives
(e.g., the one for qbraid_qir.qiskit) are missing the :members: option so module
members (classes/functions) are not included; update each API rst
(qbraid_qir.qiskit.rst, qbraid_qir.cirq.rst, qbraid_qir.qasm3.rst,
qbraid_qir.squin.rst) to add the :members: option alongside :undoc-members: and
:show-inheritance: in the automodule directive to ensure all module members are
documented.
| # Third Party Notices | ||
|
|
||
| This software includes code derived from third party sources. |
There was a problem hiding this comment.
Use “Third-Party” hyphenation for consistency with markdown grammar checks.
Line 1 and Line 3 should use “Third-Party” / “third-party”.
✏️ Proposed wording update
-# Third Party Notices
+# Third-Party Notices
...
-This software includes code derived from third party sources.
+This software includes code derived from third-party sources.🧰 Tools
🪛 LanguageTool
[grammar] ~1-~1: Use a hyphen to join words.
Context: # Third Party Notices This software includes co...
(QB_NEW_EN_HYPHEN)
[grammar] ~3-~3: Use a hyphen to join words.
Context: ...oftware includes code derived from third party sources. ## qiskit-qir The `qbra...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@NOTICE.md` around lines 1 - 3, Update the header and phrasing to use
hyphenated “Third-Party” consistently: change the heading text "Third Party
Notices" to "Third-Party Notices" and change "third party sources" to
"third-party sources" so both the title and body use the hyphenated form.
| The qiskit-qir codebase has been adapted to: | ||
| - Work with Qiskit 2.x (updated for new QuantumCircuit API) | ||
| - Follow qbraid-qir conventions and coding style | ||
| - Support both pyqir 0.10.x and 0.12+ (typed and opaque pointers) | ||
| - Integrate with the qbraid-qir module structure |
There was a problem hiding this comment.
Add a single trailing newline at end of file.
markdownlint MD047 indicates the file should end with exactly one newline.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 35-35: Files should end with a single newline character
(MD047, single-trailing-newline)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@NOTICE.md` around lines 33 - 37, NOTICE.md currently does not end with
exactly one trailing newline; update the file to ensure it finishes with a
single newline character (no extra blank lines or missing newline) so it
satisfies markdownlint MD047—open NOTICE.md, remove any extra trailing blank
lines if present, and add exactly one '\n' at EOF.
| [project.optional-dependencies] | ||
| cirq = ["cirq-core>=1.3.0,<1.6.0"] | ||
| qasm3 = ["pyqasm>=0.4.0,<1.1.0", "numpy"] | ||
| qiskit = ["qiskit>=2.0"] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest major version of Qiskit and are there breaking changes between Qiskit 2.x and 3.x?
💡 Result:
- Latest major version of Qiskit (SDK,
qiskitPyPI package): 2.x (latest release shown on PyPI is 2.3.0). [1] - Breaking changes between Qiskit 2.x and 3.x: Qiskit 3.x has not been released yet (IBM’s 2.3 release notes say they do not expect to release Qiskit 3.0 until much later in 2026), so there are no actual 2→3 breaking changes to assess today—only plans/expectations. [2]
- One explicitly noted planned change for 3.0 in the 2.3 notes:
annotated=Nonewill no longer be allowed (default would change toannotated=True, whileannotated=Falseremains available). [2]
- One explicitly noted planned change for 3.0 in the 2.3 notes:
Sources: [1] PyPI qiskit project page, [2] IBM Quantum “Qiskit SDK 2.3 release notes”.
Consider constraining the Qiskit extra to 2.x as defensive major-version pinning.
While qiskit>=2.0 would technically allow Qiskit 3.x if released with breaking changes, Qiskit 3.0 has not been released yet and is not planned until later in 2026. However, pinning to a major version constraint is a reasonable defensive practice to avoid unexpected breakage from future major releases.
📦 Suggested fix
-qiskit = ["qiskit>=2.0"]
+qiskit = ["qiskit>=2.0,<3.0"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| qiskit = ["qiskit>=2.0"] | |
| qiskit = ["qiskit>=2.0,<3.0"] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` at line 43, Update the Qiskit extra dependency specification
in pyproject.toml (the line currently reading qiskit = ["qiskit>=2.0"]) to
constrain to the 2.x series so future major releases won’t be pulled in; replace
the loose ">=2.0" spec with a 2.x‑bounded spec (e.g., >=2.0,<3.0) to pin the
major version while allowing patch/minor updates.
| from qiskit import ClassicalRegister, QuantumCircuit, QuantumRegister | ||
|
|
||
|
|
||
| @pytest.fixture() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "conftest.py" | grep qiskit_qirRepository: qBraid/qbraid-qir
Length of output: 91
🏁 Script executed:
cat -n tests/qiskit_qir/conftest.py | head -160Repository: qBraid/qbraid-qir
Length of output: 5023
🌐 Web query:
Ruff PT001 pytest fixture rule documentation
💡 Result:
Ruff’s PT001 rule is pytest-fixture-incorrect-parentheses-style. It flags argument-free @pytest.fixture decorators that use the wrong parentheses style for your configured preference (with or without ()), to keep fixture decorators consistent. By default, Ruff prefers no parentheses when there are no arguments (i.e., @pytest.fixture instead of @pytest.fixture()). [1]
- Rule doc:
pytest-fixture-incorrect-parentheses-style (PT001)[1] - Related setting:
lint.flake8-pytest-style.fixture-parentheses[1] - Rule index entry (PT001): [2]
🏁 Script executed:
wc -l tests/qiskit_qir/conftest.pyRepository: qBraid/qbraid-qir
Length of output: 93
🏁 Script executed:
cat -n tests/qiskit_qir/conftest.py | tail -20Repository: qBraid/qbraid-qir
Length of output: 756
Use bare @pytest.fixture to satisfy PT001.
All 13 fixture decorators in this file use @pytest.fixture() with parentheses but pass no arguments. Change them to bare @pytest.fixture instead.
🧹 Suggested fix pattern
-@pytest.fixture()
+@pytest.fixture
def bell_circuit():Apply to all fixtures: lines 24, 34, 45, 60, 70, 80, 88, 97, 105, 115, 125, 137, 153.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.fixture() | |
| `@pytest.fixture` |
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 24-24: Use @pytest.fixture over @pytest.fixture()
Remove parentheses
(PT001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/qiskit_qir/conftest.py` at line 24, Replace the unnecessary empty-call
fixture decorators: change every occurrence of the decorator `@pytest.fixture`()
to the bare `@pytest.fixture` for each fixture in conftest.py (all 13 fixtures
that currently use `@pytest.fixture`() — e.g., the decorators on the fixtures at
the noted locations), so the fixtures use the bare decorator form and satisfy
PT001.
| class TestSingleQubitGates: | ||
| """Tests for single qubit gate conversions.""" | ||
|
|
||
| @pytest.mark.parametrize("gate_name,qir_name", SINGLE_QUBIT_GATES.items()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd test_basic_gates.pyRepository: qBraid/qbraid-qir
Length of output: 97
🌐 Web query:
Ruff PT006 pytest parametrize argnames rule
💡 Result:
Ruff PT006 (“pytest-parametrize-names-wrong-type”) flags cases where the first argument (argnames) passed to @pytest.mark.parametrize(...) is the wrong type/style.
argnamesmust be a string (single name, or comma-separated names) or a sequence of strings (e.g.,("a", "b")). Pytest accepts both forms. [2]- Ruff enforces a consistent convention:
- Single parameter: prefer a string (e.g.,
"param"), not("param",)[1] - Multiple parameters: prefer the style configured via
lint.flake8-pytest-style.parametrize-names-type(commonly a tuple like("a", "b")). [1]
- Single parameter: prefer a string (e.g.,
Examples Ruff will complain about (depending on configuration):
`@pytest.mark.parametrize`(("param",), [1, 2, 3]) # single should be "param"
`@pytest.mark.parametrize`(["a", "b"], [(1, 2), (3, 4)]) # list vs expected tuple
`@pytest.mark.parametrize`("a,b", [(1, 2), (3, 4)]) # string vs expected tuplePreferred:
`@pytest.mark.parametrize`("param", [1, 2, 3])
`@pytest.mark.parametrize`(("a", "b"), [(1, 2), (3, 4)])[1] [2]
🏁 Script executed:
wc -l tests/qiskit_qir/test_basic_gates.pyRepository: qBraid/qbraid-qir
Length of output: 101
🏁 Script executed:
sed -n '55,60p' tests/qiskit_qir/test_basic_gates.py
sed -n '73,78p' tests/qiskit_qir/test_basic_gates.py
sed -n '111,116p' tests/qiskit_qir/test_basic_gates.pyRepository: qBraid/qbraid-qir
Length of output: 865
🏁 Script executed:
fd -e toml -e cfg | head -20 | xargs grep -l "pytest\|ruff" 2>/dev/null | head -5Repository: qBraid/qbraid-qir
Length of output: 70
🏁 Script executed:
cat ruff.tomlRepository: qBraid/qbraid-qir
Length of output: 1876
Use tuple argnames in pytest.mark.parametrize to satisfy PT006.
Lines 57, 75, and 113 use comma-separated string format for multiple parameter names. Ruff's PT006 rule requires tuple format ("gate_name", "qir_name") instead of "gate_name,qir_name".
✅ Example fix
-@pytest.mark.parametrize("gate_name,qir_name", SINGLE_QUBIT_GATES.items())
+@pytest.mark.parametrize(("gate_name", "qir_name"), SINGLE_QUBIT_GATES.items())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.mark.parametrize("gate_name,qir_name", SINGLE_QUBIT_GATES.items()) | |
| `@pytest.mark.parametrize`(("gate_name", "qir_name"), SINGLE_QUBIT_GATES.items()) |
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 57-57: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple
Use a tuple for the first argument
(PT006)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/qiskit_qir/test_basic_gates.py` at line 57, Replace comma-separated
string argnames in the pytest.mark.parametrize decorators with a tuple of
strings to satisfy PT006: change occurrences like
pytest.mark.parametrize("gate_name,qir_name", SINGLE_QUBIT_GATES.items()) to
pytest.mark.parametrize(("gate_name", "qir_name"), SINGLE_QUBIT_GATES.items())
(also apply the same tuple-style fix for the other parametrize uses at the same
file around the other decorators on lines where parametrize is used).
| ir = str(module) | ||
| assert f"__quantum__qis__{gate_name}__body" in ir | ||
| # Check that the rotation angle is included | ||
| assert "0.5" in ir or "5.0" in ir or "double" in ir |
There was a problem hiding this comment.
Strengthen rotation-angle assertion to avoid false positives.
assert "0.5" in ir or "5.0" in ir or "double" in ir is too permissive ("double" can appear regardless of angle value), so this test can pass even when the angle encoding is wrong.
🔍 Suggested assertion tightening
- assert "0.5" in ir or "5.0" in ir or "double" in ir
+ assert ("5.000000e-01" in ir) or ("0.500000" in ir)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/qiskit_qir/test_basic_gates.py` around lines 104 - 107, The current
assertion is too permissive because "double" can appear elsewhere; instead,
locate the function IR for "__quantum__qis__{gate_name}__body" (use the existing
ir = str(module) and the f"__quantum__qis__{gate_name}__body" check), then
assert that a precise angle literal appears in that function body (e.g., look
inside the function substring and require a specific LLVM constant/token such as
"double 0.5" or the exact encoding you expect like "0.5" or "5.0" within that
function body) rather than asserting "double" anywhere in the whole IR. Ensure
you reference ir, module and gate_name when extracting the function substring
and replace the broad assertion with this targeted check.
|
|
||
| def test_invalid_input_type(self): | ||
| """Test that TypeError is raised for non-QuantumCircuit input.""" | ||
| with pytest.raises(TypeError, match="must be of type qiskit.QuantumCircuit"): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and locate the test file
find . -name "test_qiskit_to_qir.py" -type fRepository: qBraid/qbraid-qir
Length of output: 101
🏁 Script executed:
# Read the specific area around line 167 in the test file
cat -n tests/qiskit_qir/test_qiskit_to_qir.py | sed -n '160,175p'Repository: qBraid/qbraid-qir
Length of output: 742
🏁 Script executed:
# Search for related error messages or patterns in the codebase
rg "must be of type qiskit.QuantumCircuit" --type pyRepository: qBraid/qbraid-qir
Length of output: 302
🏁 Script executed:
# Check if there are any linting tools configured and run them on this file
if [ -f pyproject.toml ] || [ -f setup.cfg ] || [ -f .ruff.toml ]; then
echo "Found linting config"
# Try to check for RUF043 violations
rg "ruff" pyproject.toml setup.cfg 2>/dev/null || echo "No ruff config found in standard locations"
fiRepository: qBraid/qbraid-qir
Length of output: 124
Use a raw, escaped regex in pytest.raises(..., match=...).
The pattern contains unescaped . metacharacters that will match any character instead of literal dots. For example, qiskitXQuantumCircuit would incorrectly match.
🔧 Proposed fix
- with pytest.raises(TypeError, match="must be of type qiskit.QuantumCircuit"):
+ with pytest.raises(TypeError, match=r"must be of type qiskit\.QuantumCircuit"):🧰 Tools
🪛 Ruff (0.15.2)
[warning] 167-167: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/qiskit_qir/test_qiskit_to_qir.py` at line 167, Update the pytest.raises
call in test_qiskit_to_qir.py that currently uses match="must be of type
qiskit.QuantumCircuit" to use a raw, escaped regex so the dots are treated
literally; change the match argument to a raw string with escaped dots (e.g.,
r"must be of type qiskit\.QuantumCircuit") in the pytest.raises(...) invocation
to prevent unintended matches for any-character dots.
…flow The CI was failing for two reasons: 1. Pylint violations in the new qiskit module (too-many-arguments, too-many-instance-attributes, too-many-branches, too-many-statements, consider-using-from-import, unnecessary-comprehension, consider-using-in) 2. Missing `--extra qiskit` in the test workflow, causing ModuleNotFoundError for qiskit during test collection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pushed a fix for the CI failures in b69c5d9. There were two issues: 1. Pylint violations (format-check)
All resolved with targeted fixes and pylint disable comments consistent with the existing codebase patterns (e.g. 2. Missing qiskit dependency in test workflow Verified locally — pylint scores 10/10 and all 43 qiskit tests pass. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/qiskit_qir/test_qiskit_to_qir.py (1)
167-167:⚠️ Potential issue | 🟡 MinorEscape the regex in
pytest.raisesmatch.Line 167 uses a non-raw pattern with unescaped
.characters, so the assertion can match unintended messages.🔧 Proposed fix
- with pytest.raises(TypeError, match="must be of type qiskit.QuantumCircuit"): + with pytest.raises(TypeError, match=r"must be of type qiskit\.QuantumCircuit"):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/qiskit_qir/test_qiskit_to_qir.py` at line 167, The test's pytest.raises call uses an unescaped regex for the match string ("must be of type qiskit.QuantumCircuit") so the dots match any character; update the assertion in the test (the pytest.raises invocation around the failing call) to use a raw, escaped regex for the exact type name (e.g., use a raw string with escaped dots such as r"must be of type qiskit\.QuantumCircuit" or build the pattern with re.escape("qiskit.QuantumCircuit")) so the error message match is precise.qbraid_qir/qiskit/visitor.py (1)
264-266:⚠️ Potential issue | 🟡 MinorUse strict zip for measurement pairing.
Line 264 should fail fast on mismatched qubit/result lengths instead of silently dropping items.
🔧 Proposed fix
- for qubit, result in zip(qubits, results): + for qubit, result in zip(qubits, results, strict=True):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@qbraid_qir/qiskit/visitor.py` around lines 264 - 266, The measurement loop currently uses zip(qubits, results) which silently drops pairs when lengths differ; update the code around the loop that sets self._measured_qubits and calls qis.mz to validate qubits and results lengths first (e.g. check len(qubits) == len(results) and raise a clear ValueError if not) or iterate with itertools.zip_longest and raise on a missing value, then proceed to set self._measured_qubits[pointer_id(qubit)] = True and call qis.mz(self._builder, qubit, result) for each validated pair.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@qbraid_qir/qiskit/elements.py`:
- Around line 141-143: from_circuit currently accepts module: Optional[Module]
but passes None downstream causing BasicQiskitVisitor.visit_qiskit_module to
access module.module.context and crash; update QiskitModule.from_circuit to
handle module==None by either constructing a minimal Module instance (so
module.module.context is valid) or by raising a clear ValueError early—modify
the logic in from_circuit (and the call sites around lines where QiskitModule is
created) to ensure module is never None before returning/using the QiskitModule,
and reference QiskitModule.from_circuit and
BasicQiskitVisitor.visit_qiskit_module when making the change.
---
Duplicate comments:
In `@qbraid_qir/qiskit/visitor.py`:
- Around line 264-266: The measurement loop currently uses zip(qubits, results)
which silently drops pairs when lengths differ; update the code around the loop
that sets self._measured_qubits and calls qis.mz to validate qubits and results
lengths first (e.g. check len(qubits) == len(results) and raise a clear
ValueError if not) or iterate with itertools.zip_longest and raise on a missing
value, then proceed to set self._measured_qubits[pointer_id(qubit)] = True and
call qis.mz(self._builder, qubit, result) for each validated pair.
In `@tests/qiskit_qir/test_qiskit_to_qir.py`:
- Line 167: The test's pytest.raises call uses an unescaped regex for the match
string ("must be of type qiskit.QuantumCircuit") so the dots match any
character; update the assertion in the test (the pytest.raises invocation around
the failing call) to use a raw, escaped regex for the exact type name (e.g., use
a raw string with escaped dots such as r"must be of type qiskit\.QuantumCircuit"
or build the pattern with re.escape("qiskit.QuantumCircuit")) so the error
message match is precise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 44848ed7-3207-437b-807c-94a4100d62e9
📒 Files selected for processing (4)
.github/workflows/main.ymlqbraid_qir/qiskit/elements.pyqbraid_qir/qiskit/visitor.pytests/qiskit_qir/test_qiskit_to_qir.py
| def from_circuit( | ||
| cls, circuit: QuantumCircuit, module: Optional[Module] = None | ||
| ) -> "QiskitModule": |
There was a problem hiding this comment.
Handle module=None in from_circuit to prevent runtime failure.
from_circuit advertises module as optional (Line 142), but Lines 169-173 pass None through. Downstream, BasicQiskitVisitor.visit_qiskit_module accesses module.module.context, which will fail if module is None.
🔧 Proposed fix
-from pyqir import Module
+from pyqir import Context, Module, qir_module
...
def from_circuit(
cls, circuit: QuantumCircuit, module: Optional[Module] = None
) -> "QiskitModule":
...
- name = generate_module_id(circuit)
+ name = generate_module_id(circuit)
+ if module is None:
+ module = qir_module(Context(), name)
return cls(
circuit=circuit,
name=name,
module=module,Also applies to: 167-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@qbraid_qir/qiskit/elements.py` around lines 141 - 143, from_circuit currently
accepts module: Optional[Module] but passes None downstream causing
BasicQiskitVisitor.visit_qiskit_module to access module.module.context and
crash; update QiskitModule.from_circuit to handle module==None by either
constructing a minimal Module instance (so module.module.context is valid) or by
raising a clear ValueError early—modify the logic in from_circuit (and the call
sites around lines where QiskitModule is created) to ensure module is never None
before returning/using the QiskitModule, and reference QiskitModule.from_circuit
and BasicQiskitVisitor.visit_qiskit_module when making the change.
|
@TheGupta2012 FYI: @eidrynbot Is one of my bots that I have working on a few things |
2 participants
Summary
Port of microsoft/qiskit-qir (MIT License, now archived) into qbraid-qir as
qbraid_qir.qiskit.Closes #272
Changes
New Module:
qbraid_qir.qiskitqiskit_to_qir(circuit, name, **kwargs): Main conversion function matching thecirq_to_qir/qasm3_to_qirinterfaceQiskitModule: Circuit wrapper for QIR conversionBasicQiskitVisitor: Visitor for converting Qiskit operations to QIRQiskitConversionError: Exception for conversion failuresKey Updates from Original qiskit-qir
QuantumCircuitAPI (CircuitInstructioninstead of tuples)Testing
Documentation
qbraid_qir.qiskitNOTICE.mdfor MIT license attributionConfiguration
qiskitoptional dependency in pyproject.tomlLicense Compliance
The original qiskit-qir is MIT licensed. Code derived from it includes proper attribution in file headers (following the pattern from qBraid SDK) and full license text in
NOTICE.md.Related
Summary by CodeRabbit
New Features
Documentation
Tests
Chores