From 29980c187a215161eda2fabb68816ae8ae34b382 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Fri, 21 Jun 2024 17:43:21 +0100 Subject: [PATCH] Deprecate tuple-like access to `CircuitInstruction` This has been the legacy path since `CircuitInstruction` was added in gh-8093. It's more performant to use the attribute-access patterns, and with more of the internals moving to Rust and potentially needing more use of additional class methods and attributes, we need to start shifting people away from the old form. --- crates/circuit/src/circuit_instruction.rs | 39 +++++++++++++++++-- crates/circuit/src/imports.rs | 2 + ...-circuit-instruction-8a332ab09de73766.yaml | 23 +++++++++++ test/utils/base.py | 9 +++++ 4 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/deprecate-legacy-circuit-instruction-8a332ab09de73766.yaml diff --git a/crates/circuit/src/circuit_instruction.rs b/crates/circuit/src/circuit_instruction.rs index 5179190d8aa2..ac5a2b7c3713 100644 --- a/crates/circuit/src/circuit_instruction.rs +++ b/crates/circuit/src/circuit_instruction.rs @@ -14,7 +14,7 @@ use std::cell::RefCell; use pyo3::basic::CompareOp; -use pyo3::exceptions::PyValueError; +use pyo3::exceptions::{PyDeprecationWarning, PyValueError}; use pyo3::prelude::*; use pyo3::types::{IntoPyDict, PyList, PyTuple, PyType}; use pyo3::{intern, IntoPy, PyObject, PyResult}; @@ -22,7 +22,7 @@ use smallvec::{smallvec, SmallVec}; use crate::imports::{ get_std_gate_class, populate_std_gate_map, GATE, INSTRUCTION, OPERATION, - SINGLETON_CONTROLLED_GATE, SINGLETON_GATE, + SINGLETON_CONTROLLED_GATE, SINGLETON_GATE, WARNINGS_WARN, }; use crate::interner::Index; use crate::operations::{OperationType, Param, PyGate, PyInstruction, PyOperation, StandardGate}; @@ -564,26 +564,31 @@ impl CircuitInstruction { #[cfg(not(feature = "cache_pygates"))] pub fn __getitem__(&self, py: Python<'_>, key: &Bound) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; Ok(self._legacy_format(py)?.as_any().get_item(key)?.into_py(py)) } #[cfg(feature = "cache_pygates")] pub fn __getitem__(&mut self, py: Python<'_>, key: &Bound) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; Ok(self._legacy_format(py)?.as_any().get_item(key)?.into_py(py)) } #[cfg(not(feature = "cache_pygates"))] pub fn __iter__(&self, py: Python<'_>) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; Ok(self._legacy_format(py)?.as_any().iter()?.into_py(py)) } #[cfg(feature = "cache_pygates")] pub fn __iter__(&mut self, py: Python<'_>) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; Ok(self._legacy_format(py)?.as_any().iter()?.into_py(py)) } - pub fn __len__(&self) -> usize { - 3 + pub fn __len__(&self, py: Python) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; + Ok(3) } pub fn __richcmp__( @@ -931,3 +936,29 @@ pub(crate) fn convert_py_to_operation_type( } Err(PyValueError::new_err(format!("Invalid input: {}", py_op))) } + +/// Issue a Python `DeprecationWarning` about using the legacy tuple-like interface to +/// `CircuitInstruction`. +/// +/// Beware the `stacklevel` here doesn't work quite the same way as it does in Python as Rust-space +/// calls are completely transparent to Python. +#[inline] +fn warn_on_legacy_circuit_instruction_iteration(py: Python) -> PyResult<()> { + WARNINGS_WARN + .get_bound(py) + .call1(( + intern!( + py, + concat!( + "Treating CircuitInstruction as an iterable is deprecated legacy behavior", + " since Qiskit 1.2, and will be removed in Qiskit 2.0.", + " Instead, use the `operation`, `qubits` and `clbits` named attributes." + ) + ), + py.get_type_bound::(), + // Stack level. Compared to Python-space calls to `warn`, this is unusually low + // beacuse all our internal call structure is now Rust-space and invisible to Python. + 1, + )) + .map(|_| ()) +} diff --git a/crates/circuit/src/imports.rs b/crates/circuit/src/imports.rs index 8db3b88fd7d2..2e537b6bf6d9 100644 --- a/crates/circuit/src/imports.rs +++ b/crates/circuit/src/imports.rs @@ -72,6 +72,8 @@ pub static SINGLETON_GATE: ImportOnceCell = pub static SINGLETON_CONTROLLED_GATE: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.singleton", "SingletonControlledGate"); +pub static WARNINGS_WARN: ImportOnceCell = ImportOnceCell::new("warnings", "warn"); + /// A mapping from the enum variant in crate::operations::StandardGate to the python /// module path and class name to import it. This is used to populate the conversion table /// when a gate is added directly via the StandardGate path and there isn't a Python object diff --git a/releasenotes/notes/deprecate-legacy-circuit-instruction-8a332ab09de73766.yaml b/releasenotes/notes/deprecate-legacy-circuit-instruction-8a332ab09de73766.yaml new file mode 100644 index 000000000000..d656ee5cb823 --- /dev/null +++ b/releasenotes/notes/deprecate-legacy-circuit-instruction-8a332ab09de73766.yaml @@ -0,0 +1,23 @@ +--- +deprecations_circuits: + - | + Treating :class:`.CircuitInstruction` as a tuple-like iterable is deprecated, and this legacy + path way will be removed in Qiskit 2.0. You should use the attribute-access fields + :attr:`~.CircuitInstruction.operation`, :attr:`~.CircuitInstruction.qubits`, and + :attr:`~.CircuitInstruction.clbits` instead. For example:: + + from qiskit.circuit import QuantumCircuit + + qc = QuantumCircuit(2, 2) + qc.h(0) + qc.cx(0, 1) + qc.measure([0, 1], [0, 1]) + + # Deprecated. + for op, qubits, clbits in qc.data: + pass + # New style. + for instruction in qc.data: + op = instruction.operation + qubits = instruction.qubits + clbits = instruction.clbits diff --git a/test/utils/base.py b/test/utils/base.py index 63a8bf4384f0..bebf03008858 100644 --- a/test/utils/base.py +++ b/test/utils/base.py @@ -215,6 +215,15 @@ def setUpClass(cls): module=r"seaborn(\..*)?", ) + # Safe to remove once https://github.com/Qiskit/qiskit-aer/pull/2179 is in a release version + # of Aer. + warnings.filterwarnings( + "default", + category=DeprecationWarning, + message="Treating CircuitInstruction as an iterable is deprecated", + module=r"qiskit_aer(\.[a-zA-Z0-9_]+)*", + ) + allow_DeprecationWarning_modules = [ "test.python.pulse.test_builder", "test.python.pulse.test_block",