Skip to content

Commit

Permalink
Deprecate tuple-like access to CircuitInstruction (Qiskit#12640)
Browse files Browse the repository at this point in the history
This has been the legacy path since `CircuitInstruction` was added in
Qiskitgh-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.
  • Loading branch information
jakelishman authored Jun 24, 2024
1 parent b20a7ce commit 8b1f75f
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 5 deletions.
39 changes: 35 additions & 4 deletions crates/circuit/src/circuit_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
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};
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};
Expand Down Expand Up @@ -572,26 +572,31 @@ impl CircuitInstruction {

#[cfg(not(feature = "cache_pygates"))]
pub fn __getitem__(&self, py: Python<'_>, key: &Bound<PyAny>) -> PyResult<PyObject> {
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<PyAny>) -> PyResult<PyObject> {
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<PyObject> {
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<PyObject> {
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<usize> {
warn_on_legacy_circuit_instruction_iteration(py)?;
Ok(3)
}

pub fn __richcmp__(
Expand Down Expand Up @@ -939,3 +944,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::<PyDeprecationWarning>(),
// 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(|_| ())
}
2 changes: 2 additions & 0 deletions crates/circuit/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
6 changes: 5 additions & 1 deletion test/python/circuit/test_circuit_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,11 @@ def to_legacy(instruction):
return (instruction.operation, list(instruction.qubits), list(instruction.clbits))

expected = [to_legacy(instruction) for instruction in qc.data]
actual = [tuple(instruction) for instruction in qc.data]

with self.assertWarnsRegex(
DeprecationWarning, "Treating CircuitInstruction as an iterable is deprecated"
):
actual = [tuple(instruction) for instruction in qc.data]
self.assertEqual(actual, expected)

def test_getitem_by_insertion_order(self):
Expand Down
9 changes: 9 additions & 0 deletions test/utils/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 8b1f75f

Please sign in to comment.