From 274b13c409deecc3f8c244ee2c6b540958f93e35 Mon Sep 17 00:00:00 2001 From: Ryan Hill Date: Fri, 7 Aug 2026 08:17:00 -0500 Subject: [PATCH 1/3] fix: use presence tests for loads() kwargs and reject invalid ones Six documented loads() kwargs were stored behind a walrus truthiness test, so falsy caller values were silently discarded. All kwargs now use presence tests; unknown kwarg names raise TypeError and non-positive numeric values raise ValueError, so mistakes fail at the call site. Fixes #356 --- CHANGELOG.md | 2 + src/pyqasm/entrypoint.py | 55 +++++++++++++++++++-------- tests/test_entrypoint.py | 81 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 16 deletions(-) create mode 100644 tests/test_entrypoint.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 864ef9d7..218c64f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,12 +22,14 @@ Types of changes: - Consolidated the hardcoded `"__PYQASM_QUBITS__"` string literals scattered across `visitor.py`, `transformer.py` and `pulse/utils.py` into a single `INTERNAL_QUBIT_REGISTER` constant in `elements.py`, alongside an `is_internal_qubit_register()` helper that is now the one place the internal register is recognised. ([#325](https://github.com/qBraid/pyqasm/pull/325)) - Added / updated type hinting for `base.py`, `qasm2.py`, `qasm3.py`, and `visitor.py` signatures, plus the `visit_statement` / `visit_basic_block` signatures in `pulse/visitor.py`. Fixed grammatical typos in `base.py`, `qasm2.py`, `qasm3.py`, and `visitor.py` docstrings. Fixes incorrect return types in the docstrings of `visitor.py`. ([#346](https://github.com/qBraid/pyqasm/pull/346)) - Added / updated type hinting for `pulse/visitor.py` signatures and fixes incorrect return types in the docstrings of `pulse/visitor.py`. ([#365](https://github.com/qBraid/pyqasm/pull/365)) +- **Breaking:** `loads()` and `load()` now raise `TypeError` on an unrecognised keyword argument instead of silently ignoring it, so a misspelling like `loads(src, devise_qubits=5)` fails at the call site rather than being dropped. Any caller currently passing a stray or misspelled kwarg will break on upgrade. ([#356](https://github.com/qBraid/pyqasm/issues/356)) ### Deprecated ### Removed ### Fixed +- Fixed `loads()` silently discarding a falsy kwarg value. The seven documented kwargs were stored behind a walrus truthiness test, so a caller value of `0`, `0.0`, `False`, `{}` or `[]` was treated as though the kwarg had never been passed. Each kwarg is now tested for presence, with an explicit `None` still meaning "not passed". What that changes per kwarg: `extern_functions={}` / `[]` is now stored rather than dropped; a non-positive `device_qubits` / `device_cycle_time` / `compiler_angle_type_size` / `frame_limit_per_port` is now **rejected** with a `ValueError` at the call site rather than silently dropped, along with a non-numeric or `bool` value (`TypeError`); and `frame_in_def_cal=None` / `play_in_cal_block=None` now mean "not passed" — previously an explicit `None` was stored and read as falsy, so passing `None` disabled the check that the `True` default enables. Pass `False` for the old `None` behaviour. ([#356](https://github.com/qBraid/pyqasm/issues/356)) - Fixed inaccurate `device_qubits` entry in `QasmModule.unroll()` docstring ([#349](https://github.com/qBraid/pyqasm/pull/349)) - 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)) diff --git a/src/pyqasm/entrypoint.py b/src/pyqasm/entrypoint.py index 73af7916..04f40eb7 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,10 @@ 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 + for name, attr in _LOADS_KWARG_ATTRS.items(): + if name in kwargs: + setattr(module, attr, kwargs[name]) return module diff --git a/tests/test_entrypoint.py b/tests/test_entrypoint.py new file mode 100644 index 00000000..e6d7e9a7 --- /dev/null +++ b/tests/test_entrypoint.py @@ -0,0 +1,81 @@ +# 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 + + +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) From a064938f9c309581660ebff445ef912c9f7c40fd Mon Sep 17 00:00:00 2001 From: Ryan Hill Date: Fri, 7 Aug 2026 11:26:13 -0500 Subject: [PATCH 2/3] fix: treat explicit None kwargs as omitted in loads() (Argus P1) An explicit None no longer clobbers non-None defaults like extern_functions={} or frame_in_def_cal=True. --- src/pyqasm/entrypoint.py | 6 ++++-- tests/test_entrypoint.py | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/pyqasm/entrypoint.py b/src/pyqasm/entrypoint.py index 04f40eb7..3965318e 100644 --- a/src/pyqasm/entrypoint.py +++ b/src/pyqasm/entrypoint.py @@ -133,9 +133,11 @@ def loads(program: openqasm3.ast.Program | str, **kwargs) -> QasmModule: qasm_module = Qasm3Module if program.version.startswith("3") else Qasm2Module module = qasm_module("main", program) - # presence tests, not truthiness: a falsy value is a caller value, not an omission + # 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 name in kwargs: + 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 index e6d7e9a7..e4b82efa 100644 --- a/tests/test_entrypoint.py +++ b/tests/test_entrypoint.py @@ -50,6 +50,9 @@ def test_loads_kwargs_are_stored(kwarg, 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(): From 81b764d2315357f86ed3c55e39a22b6ab260a6df Mon Sep 17 00:00:00 2001 From: Ryan Hill Date: Thu, 13 Aug 2026 15:23:36 -0500 Subject: [PATCH 3/3] fix: type-check numeric loads() kwargs, name the calling entrypoint Address review on #361: - _validate_kwargs guards the type before comparing. A non-numeric value raised a bare "'<=' not supported between instances of 'str' and 'int'" naming neither the kwarg nor the function, and bool slipped through positivity so device_qubits=True stored as a count of 1. - load() validates too, so its messages name load() rather than loads(), and its docstring gains the Raises section and a pointer to loads(). - Document that an explicit None means "not passed" for every kwarg, and record the frame_in_def_cal/play_in_cal_block None change in the changelog -- an explicit None used to be stored and read as falsy, so it disabled the check the True default enables. Pass False for that. - Rewrite the changelog entry to say non-positive values are rejected rather than honoured, and move the unknown-kwarg TypeError to Improved / Modified marked breaking. - Drop the redundant 'name in kwargs', reword the is-not-None comment, and assert the target attribute exists before setattr. - Cover "5", [], complex, {} and bool in tests, plus load()'s messages. --- src/pyqasm/entrypoint.py | 52 ++++++++++++++++++++++++++++++++-------- tests/test_entrypoint.py | 43 +++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/src/pyqasm/entrypoint.py b/src/pyqasm/entrypoint.py index 3965318e..810fbc68 100644 --- a/src/pyqasm/entrypoint.py +++ b/src/pyqasm/entrypoint.py @@ -52,16 +52,31 @@ ) -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).""" +def _validate_kwargs(kwargs: dict, func: str = "loads") -> None: + """Reject unknown kwarg names and unusable values at the call site, instead of + silently dropping them (issue #356). + + Args: + kwargs (dict): The keyword arguments the caller passed. + func (str): The entrypoint to name in error messages. + + Raises: + TypeError: If a kwarg name is unrecognised, or a positive-only kwarg is not + a real number. + ValueError: If a positive-only kwarg is zero or negative. + """ unknown = sorted(set(kwargs) - set(_LOADS_KWARG_ATTRS)) if unknown: - raise TypeError(f"loads() got unexpected keyword argument(s): {', '.join(unknown)}") + raise TypeError(f"{func}() 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}") + if value is None: + continue + # bool is a subclass of int, so True would otherwise pass as a count of 1 + if isinstance(value, bool) or not isinstance(value, (int, float)): + raise TypeError(f"{func}() kwarg '{name}' must be a number, got {type(value).__name__}") + if value <= 0: + raise ValueError(f"{func}() kwarg '{name}' must be positive, got {value!r}") def load(filename: str, **kwargs) -> QasmModule: @@ -70,6 +85,15 @@ def load(filename: str, **kwargs) -> QasmModule: Args: filename (str): The filename of the OpenQASM program to validate. + **kwargs: Forwarded to :func:`loads`; see it for the supported names. + + Raises: + TypeError: If ``filename`` is not a string, or if an unrecognized keyword + argument is passed. + FileNotFoundError: If the file does not exist. + 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 @@ -78,6 +102,8 @@ def load(filename: str, **kwargs) -> QasmModule: raise TypeError("Input 'filename' must be of type 'str'.") if not os.path.isfile(filename): raise FileNotFoundError(f"QASM file '{filename}' not found.") + # validate here as well so the message names load(), the function the caller invoked + _validate_kwargs(kwargs, func="load") program = process_include_statements(filename) return loads(program, **kwargs) @@ -104,9 +130,13 @@ def loads(program: openqasm3.ast.Program | str, **kwargs) -> QasmModule: - **play_in_cal_block** (bool): Whether to allow play in defcal. + Passing an explicit ``None`` for any of these means "not passed": the + module default is kept. Pass ``False`` to turn off a boolean kwarg. + Raises: TypeError: If the input is not a string or an `openqasm3.ast.Program` instance, - or if an unrecognized keyword argument is passed. + if an unrecognized keyword argument is passed, or if a numeric keyword + argument is not a real number. ValueError: If a numeric keyword argument is zero or negative. ValidationError: If the program fails parsing or semantic validation. @@ -133,11 +163,13 @@ def loads(program: openqasm3.ast.Program | str, **kwargs) -> QasmModule: qasm_module = Qasm3Module if program.version.startswith("3") else Qasm2Module module = qasm_module("main", program) - # 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. + # `is not None`, not truthiness: a falsy value is a caller value, not an omission. + # An explicit None means "not passed", so defaults like extern_functions={} and + # frame_in_def_cal=True are never clobbered. for name, attr in _LOADS_KWARG_ATTRS.items(): if kwargs.get(name) is not None: + # setattr would happily create a phantom attribute if the module renamed one + assert hasattr(module, attr), f"module has no attribute '{attr}' for kwarg '{name}'" setattr(module, attr, kwargs[name]) return module diff --git a/tests/test_entrypoint.py b/tests/test_entrypoint.py index e4b82efa..d4907d85 100644 --- a/tests/test_entrypoint.py +++ b/tests/test_entrypoint.py @@ -19,7 +19,7 @@ import pytest -from pyqasm.entrypoint import loads +from pyqasm.entrypoint import load, loads QASM = """ OPENQASM 3.0; @@ -51,7 +51,8 @@ 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 == {} + extern_functions = loads(QASM, extern_functions=None)._extern_functions + assert isinstance(extern_functions, dict) and not extern_functions assert loads(QASM, frame_in_def_cal=None)._frame_in_def_cal is True @@ -78,7 +79,45 @@ def test_loads_rejects_non_positive_values(kwarg, value): loads(QASM, **{kwarg: value}) +@pytest.mark.parametrize( + "kwarg, value", + [ + ("device_qubits", "5"), + ("device_qubits", []), + ("device_qubits", complex(1)), + ("device_cycle_time", {}), + ("frame_limit_per_port", "2"), + ], +) +def test_loads_rejects_non_numeric_values(kwarg, value): + """A non-numeric value must name the kwarg rather than surfacing as a bare + comparison error from inside the validator (issue #356).""" + with pytest.raises(TypeError, match=kwarg): + loads(QASM, **{kwarg: value}) + + +@pytest.mark.parametrize("kwarg", ["device_qubits", "compiler_angle_type_size"]) +@pytest.mark.parametrize("value", [True, False]) +def test_loads_rejects_bool_for_numeric_kwargs(kwarg, value): + """bool is a subclass of int, so True would otherwise pass positivity and be + stored as a count of 1, while False would report 'must be positive'.""" + with pytest.raises(TypeError, 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) + + +def test_load_errors_name_load_not_loads(tmp_path): + """load() forwards **kwargs, so its errors must name the function the caller + actually invoked.""" + path = tmp_path / "prog.qasm" + path.write_text(QASM, encoding="utf-8") + + with pytest.raises(TypeError, match=r"load\(\) got unexpected keyword argument"): + load(str(path), devise_qubits=5) + with pytest.raises(ValueError, match=r"load\(\) kwarg 'device_qubits'"): + load(str(path), device_qubits=0)