Closed lillian542 closed 7 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 100.00%. Comparing base (
265039f
) to head (fb1abf1
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
We inadvertently changed the behaviour of the QASM conversion without warning when updating the Qiskit conversion, because the QASM conversion works by converting from QASM to Qiskit, and then using the main
load
function. The change was that in the past, the measurements at the end of a QASM string have not queued. Example at the end for clarity.The solution in this PR is to slightly modify how the
measurements
kwarg is handled, so that an emptyIterable
is not treated asNone
, but is instead treated as overriding the terminal measurements with an (empty) list of measurements. We can then passmeasurements=[]
toload
to preserve the old behaviour of ignoring terminal measurements without needing to overwrite them with anything.The downside is that some users might expect passing
[]
to do the same thing as passingNone
, but to be honest I don't think this is less clear than the current implementation. Currently we say:So the question is: is
[]
None-like and therefore behaves asNone
, or is it alist
of measurements that overrides the terminal measurements? I think this behaviour is actually slightly more intuitive and allows users to easily recreate the legacy behaviour in existing code by passing an empty list to measurements.It's in the main
load
function, so it will also affect Qiskit circuit conversion.Note: the tests actually did catch this when it changed, we just misunderstood why the test results had changed and updated the tests instead of realizing the test failures meant something was wrong. Oops.
Example:
Where previously converting would give: