Skip to content

Commit

Permalink
Deprecate passing None to DAGCircuit appenders (Qiskit#10752)
Browse files Browse the repository at this point in the history
This has been against the documented typing of the functions for some
time, but some scheduling methods have mistakenly been passing `None`.
There is no need to support `None` here; the empty tuple `()` is equally
immutable and a CPython singleton, so there are neither mutability nor
memory benefits to supporting both, and removing `None` as an input is a
simple way to remove a(n admittedly cheap) branch from the appender
methods.

Co-authored-by: Luciano Bello <[email protected]>
  • Loading branch information
jakelishman and 1ucian0 authored Oct 18, 2023
1 parent d86e708 commit b18faa3
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 5 deletions.
34 changes: 30 additions & 4 deletions qiskit/dagcircuit/dagcircuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from collections import OrderedDict, defaultdict, deque, namedtuple
import copy
import math
import warnings
from typing import Dict, Generator, Any, List

import numpy as np
Expand Down Expand Up @@ -656,8 +657,16 @@ def apply_operation_back(self, op, qargs=(), cargs=(), *, check=True):
DAGCircuitError: if a leaf node is connected to multiple outputs
"""
qargs = tuple(qargs) if qargs is not None else ()
cargs = tuple(cargs) if cargs is not None else ()
if qargs is None:
_warn_none_args()
qargs = ()
else:
qargs = tuple(qargs)
if cargs is None:
_warn_none_args()
cargs = ()
else:
cargs = tuple(cargs)

if self._operation_may_have_bits(op):
# This is the slow path; most of the time, this won't happen.
Expand Down Expand Up @@ -701,8 +710,16 @@ def apply_operation_front(self, op, qargs=(), cargs=(), *, check=True):
Raises:
DAGCircuitError: if initial nodes connected to multiple out edges
"""
qargs = tuple(qargs) if qargs is not None else ()
cargs = tuple(cargs) if cargs is not None else ()
if qargs is None:
_warn_none_args()
qargs = ()
else:
qargs = tuple(qargs)
if cargs is None:
_warn_none_args()
cargs = ()
else:
cargs = tuple(cargs)

if self._operation_may_have_bits(op):
# This is the slow path; most of the time, this won't happen.
Expand Down Expand Up @@ -2099,3 +2116,12 @@ def draw(self, scale=0.7, filename=None, style="color"):
from qiskit.visualization.dag_visualization import dag_drawer

return dag_drawer(dag=self, scale=scale, filename=filename, style=style)


def _warn_none_args():
warnings.warn(
"Passing 'None' as the qubits or clbits of an operation to 'DAGCircuit' methods"
" is deprecated since Qiskit 0.45 and will be removed in Qiskit 1.0. Instead, pass '()'.",
DeprecationWarning,
stacklevel=3,
)
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def _apply_scheduled_op(
t_start: int,
oper: Instruction,
qubits: Qubit | Iterable[Qubit],
clbits: Clbit | Iterable[Clbit] | None = None,
clbits: Clbit | Iterable[Clbit] = (),
):
"""Add new operation to DAG with scheduled information.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
deprecations:
- |
Passing ``None`` as the ``qargs`` or ``cargs`` arguments to :meth:`.DAGCircuit.apply_operation_back`
or :meth:`~.DAGCircuit.apply_operation_front` is deprecated and will be removed in Qiskit 1.0.
This has been explicitly against the typing documentation for some time, but silently accepted
by Qiskit. Instead, simply pass ``()`` rather than ``None``.
12 changes: 12 additions & 0 deletions test/python/dagcircuit/test_dagcircuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,18 @@ def test_apply_operation_back(self):
self.assertEqual(len(list(self.dag.nodes())), 16)
self.assertEqual(len(list(self.dag.edges())), 17)

def test_apply_operation_rejects_none(self):
"""Test that the ``apply_operation_*`` methods warn when given ``None``."""
noop = Instruction("noop", 0, 0, [])
with self.assertWarnsRegex(DeprecationWarning, "Passing 'None'"):
self.dag.apply_operation_back(noop, None, ())
with self.assertWarnsRegex(DeprecationWarning, "Passing 'None'"):
self.dag.apply_operation_back(noop, (), None)
with self.assertWarnsRegex(DeprecationWarning, "Passing 'None'"):
self.dag.apply_operation_front(noop, None, ())
with self.assertWarnsRegex(DeprecationWarning, "Passing 'None'"):
self.dag.apply_operation_front(noop, (), None)

def test_edges(self):
"""Test that DAGCircuit.edges() behaves as expected with ops."""
x_gate = XGate().c_if(*self.condition)
Expand Down

0 comments on commit b18faa3

Please sign in to comment.