fix: use presence tests for loads() kwargs and reject invalid ones

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))

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
 
 

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)