Repository navigation
loads() silently drops falsy kwarg values (device_qubits=0, device_cycle_time=0.0, ...) #356
Description
Description
loads() stores six of its documented kwargs with a walrus truthiness test rather than a presence test:
if dev_qbts := kwargs.get("device_qubits"):
module._device_qubits = dev_qbtsSo any falsy value is silently treated as "not passed" — no error, no warning, the caller's argument is discarded. entrypoint.py:103-116.
The same function already uses the correct pattern for its two bool kwargs:
if "frame_in_def_cal" in kwargs:
module._frame_in_def_cal = kwargs["frame_in_def_cal"]Reproduction
from pyqasm import loads
src = 'OPENQASM 3.0;\ninclude "stdgates.inc";\nqubit[2] q;\nh q[0];\n'
loads(src, device_qubits=0)._device_qubits # None <- expected 0
loads(src, device_cycle_time=0.0)._device_cycle_time # None <- expected 0.0
loads(src, compiler_angle_type_size=0)._compiler_angle_type_size # None
loads(src, frame_limit_per_port=0)._frame_limit_per_port # None
loads(src, frame_in_def_cal=False)._frame_in_def_cal # False <- correctly honouredScope
Pre-existing on main; unrelated to any open PR.
extern_functions={} is affected by the same test but is harmless in practice — the attribute already defaults to {}, so the dropped value is indistinguishable from the default. The four Optional[...] = None attributes above are the ones where a caller's 0 becomes None.
Two adjacent gaps in the same block, worth deciding on together:
-
No value validation. Negative values are truthy, so they are stored and surface later as a confusing message rather than being rejected at the call site:
m = loads(src, device_qubits=-5) m.validate() # ValidationError: Total qubits '2' exceed device qubits '-5'.
-
Unknown kwargs are silently ignored.
loads(src, devise_qubits=5)raises nothing and produces a module with_device_qubits = None. A typo in a documented kwarg is invisible until the resulting behaviour is wrong.
Suggested fix
Use presence tests consistently for all of them:
if "device_qubits" in kwargs:
module._device_qubits = kwargs["device_qubits"]Optionally reject non-positive device_qubits / device_cycle_time / frame_limit_per_port at load time, and raise on unrecognised kwarg names, so mistakes fail where they are made.
Found while verifying #349, which documents device_qubits as a loads() kwarg.
Activity
fix: use presence tests for loads() kwargs and reject invalid ones
fix: use presence tests for loads() kwargs and reject invalid ones (#361