diff --git a/CHANGELOG.md b/CHANGELOG.md index bcc32f0..0f781df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Types of changes: ### Removed ### Fixed +- Fixed `loads()` silently dropping falsy kwarg values (e.g. `extern_functions={}`) by testing truthiness instead of presence. `loads()` now also rejects unknown kwarg names with a `TypeError` and non-positive `device_qubits` / `device_cycle_time` / `compiler_angle_type_size` / `frame_limit_per_port` with a `ValueError`, so mistakes fail at the call site. ([#356](https://github.com/qBraid/pyqasm/issues/356)) - Fixed `remove_idle_qubits()` and `reverse_qubit_order()` ignoring statements nested inside `box` and `if` blocks. Top-level operands were rewritten while nested ones kept their old indices, so the result silently addressed the wrong qubits — and when a nested index fell outside the shrunken register, the output was not a loadable program at all. Both passes now walk nested bodies, as do `has_measurements()` / `remove_measurements()` and `has_barriers()` / `remove_barriers()`; a box left empty by a removal is dropped, since pyqasm rejects a box with no statements. Two consequences of the same blind spot are fixed alongside: a qubit operated on only inside an `if` block no longer counts as idle, and `remove_idle_qubits()` no longer raises `AssertionError` on a program that mixes physical qubits with declared registers. ([#345](https://github.com/qBraid/pyqasm/pull/345)) - Fixed `unroll(consolidate_qubits=True)` raising `AttributeError: 'str' object has no attribute 'name'` for any gate applied to a physical qubit, e.g. `h $1;`. Consolidation assumed every gate operand was an `IndexedIdentifier`, but a physical qubit survives unrolling as `Identifier("$1")`. Physical qubits are absolute hardware indices belonging to no declared register, so they are now left as written — matching how `measure`, `reset` and `barrier` already treat them. ([#344](https://github.com/qBraid/pyqasm/pull/344)) - Fixed `unroll()` and `rebase()` emitting statements that share operand AST nodes: gate decompositions passed the same `IndexedIdentifier` objects into every statement they emitted, so transformations that rewrite qubit indices in place mutated a shared node once per referencing statement. This crashed `reverse_qubit_order()` (`KeyError: -1`) and `remove_idle_qubits()` (`KeyError`, [#331](https://github.com/qBraid/pyqasm/issues/331)) on any decomposed gate (e.g. `crz`) whenever the remap was not the identity. Statement constructors in `maps/gates.py` and `Decomposer` now copy their qubit operands so every emitted statement owns its nodes. ([#333](https://github.com/qBraid/pyqasm/issues/333)) diff --git a/src/pyqasm/entrypoint.py b/src/pyqasm/entrypoint.py index 73af791..3965318 100644 --- a/src/pyqasm/entrypoint.py +++ b/src/pyqasm/entrypoint.py @@ -32,6 +32,37 @@ if TYPE_CHECKING: import openqasm3.ast +# maps each documented loads() kwarg to the module attribute that stores it +_LOADS_KWARG_ATTRS = { + "device_qubits": "_device_qubits", + "device_cycle_time": "_device_cycle_time", + "compiler_angle_type_size": "_compiler_angle_type_size", + "extern_functions": "_extern_functions", + "frame_in_def_cal": "_frame_in_def_cal", + "frame_limit_per_port": "_frame_limit_per_port", + "play_in_cal_block": "_play_in_cal", +} + +# kwargs that must be positive when given; an explicit None counts as not given +_POSITIVE_KWARGS = ( + "device_qubits", + "device_cycle_time", + "compiler_angle_type_size", + "frame_limit_per_port", +) + + +def _validate_kwargs(kwargs: dict) -> None: + """Reject unknown kwarg names and non-positive values at the call site, + instead of silently dropping them (issue #356).""" + unknown = sorted(set(kwargs) - set(_LOADS_KWARG_ATTRS)) + if unknown: + raise TypeError(f"loads() got unexpected keyword argument(s): {', '.join(unknown)}") + for name in _POSITIVE_KWARGS: + value = kwargs.get(name) + if value is not None and name in kwargs and value <= 0: + raise ValueError(f"loads() kwarg '{name}' must be positive, got {value!r}") + def load(filename: str, **kwargs) -> QasmModule: """Loads an OpenQASM program into a `QasmModule` object. @@ -74,13 +105,16 @@ def loads(program: openqasm3.ast.Program | str, **kwargs) -> QasmModule: - **play_in_cal_block** (bool): Whether to allow play in defcal. Raises: - TypeError: If the input is not a string or an `openqasm3.ast.Program` instance. + TypeError: If the input is not a string or an `openqasm3.ast.Program` instance, + or if an unrecognized keyword argument is passed. + ValueError: If a numeric keyword argument is zero or negative. ValidationError: If the program fails parsing or semantic validation. Returns: QasmModule: An object containing the parsed qasm representation along with some useful metadata and methods """ + _validate_kwargs(kwargs) if isinstance(program, str): try: program = openqasm3.parse(program) @@ -99,21 +133,12 @@ def loads(program: openqasm3.ast.Program | str, **kwargs) -> QasmModule: qasm_module = Qasm3Module if program.version.startswith("3") else Qasm2Module module = qasm_module("main", program) - # Store device_qubits on the module for later use - if dev_qbts := kwargs.get("device_qubits"): - module._device_qubits = dev_qbts - if dev_cycle_time := kwargs.get("device_cycle_time"): - module._device_cycle_time = dev_cycle_time - if compiler_angle_type_size := kwargs.get("compiler_angle_type_size"): - module._compiler_angle_type_size = compiler_angle_type_size - if extern_functions := kwargs.get("extern_functions"): - module._extern_functions = extern_functions - if "frame_in_def_cal" in kwargs: - module._frame_in_def_cal = kwargs["frame_in_def_cal"] - if frame_limit_per_port := kwargs.get("frame_limit_per_port"): - module._frame_limit_per_port = frame_limit_per_port - if "play_in_cal_block" in kwargs: - module._play_in_cal = kwargs["play_in_cal_block"] + # presence tests, not truthiness: a falsy value is a caller value, not an omission. + # An explicit None still means "not passed", so defaults like extern_functions={} + # are never clobbered. + for name, attr in _LOADS_KWARG_ATTRS.items(): + if kwargs.get(name) is not None: + setattr(module, attr, kwargs[name]) return module diff --git a/tests/test_entrypoint.py b/tests/test_entrypoint.py new file mode 100644 index 0000000..e4b82ef --- /dev/null +++ b/tests/test_entrypoint.py @@ -0,0 +1,84 @@ +# Copyright 2025 qBraid +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Module containing unit tests for the loads() kwargs (issue #356). + +""" + +import pytest + +from pyqasm.entrypoint import loads + +QASM = """ +OPENQASM 3.0; +include "stdgates.inc"; +qubit[2] q; +h q[0]; +""" + + +@pytest.mark.parametrize( + "kwarg, attr, value", + [ + ("device_qubits", "_device_qubits", 5), + ("device_cycle_time", "_device_cycle_time", 1e-9), + ("compiler_angle_type_size", "_compiler_angle_type_size", 32), + ("extern_functions", "_extern_functions", {"f": (["int"], "int")}), + ("frame_in_def_cal", "_frame_in_def_cal", False), + ("frame_limit_per_port", "_frame_limit_per_port", 2), + ("play_in_cal_block", "_play_in_cal", False), + ], +) +def test_loads_kwargs_are_stored(kwarg, attr, value): + """Every documented kwarg must be stored on the module, falsy values included.""" + module = loads(QASM, **{kwarg: value}) + assert getattr(module, attr) == value + + +def test_loads_kwarg_none_means_not_passed(): + """An explicit None leaves the attribute at its default.""" + assert loads(QASM, device_qubits=None)._device_qubits is None + # defaults that are not None must survive an explicit None + assert loads(QASM, extern_functions=None)._extern_functions == {} + assert loads(QASM, frame_in_def_cal=None)._frame_in_def_cal is True + + +def test_loads_empty_extern_functions_is_stored(): + """A falsy dict is a caller value, not an omission.""" + extern_functions = loads(QASM, extern_functions={})._extern_functions + assert isinstance(extern_functions, dict) and not extern_functions + + +@pytest.mark.parametrize( + "kwarg, value", + [ + ("device_qubits", 0), + ("device_qubits", -5), + ("device_cycle_time", 0.0), + ("compiler_angle_type_size", 0), + ("frame_limit_per_port", -1), + ], +) +def test_loads_rejects_non_positive_values(kwarg, value): + """Zero or negative values are rejected at the call site instead of surfacing + later as a confusing validation message (issue #356).""" + with pytest.raises(ValueError, match=kwarg): + loads(QASM, **{kwarg: value}) + + +def test_loads_rejects_unknown_kwargs(): + """A typo in a kwarg name must fail where it is made, not silently do nothing.""" + with pytest.raises(TypeError, match="devise_qubits"): + loads(QASM, devise_qubits=5)