Skip to content

Commit

Permalink
Deprecate passing None to DAGCircuit appenders
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.
  • Loading branch information
jakelishman committed Aug 31, 2023
1 parent 48cfab6 commit f14012e
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 3 deletions.
33 changes: 31 additions & 2 deletions qiskit/dagcircuit/dagcircuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import copy
import itertools
import math
import warnings
from typing import Dict, Generator, Any, List

import numpy as np
Expand Down Expand Up @@ -661,8 +662,16 @@ def apply_operation_back(self, op, qargs=(), cargs=()):
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)

all_cbits = set(self._bits_in_operation(op)).union(cargs)

Expand Down Expand Up @@ -695,6 +704,17 @@ def apply_operation_front(self, op, qargs=(), cargs=()):
Raises:
DAGCircuitError: if initial nodes connected to multiple out edges
"""
if qargs is None:
_warn_none_args()
qargs = ()
else:
qargs = tuple(qargs)
if cargs is None:
_warn_none_args()
cargs = ()
else:
cargs = tuple(cargs)

all_cbits = set(self._bits_in_operation(op)).union(cargs)

self._check_condition(op.name, getattr(op, "condition", None))
Expand Down Expand Up @@ -2071,3 +2091,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 @@ -488,6 +488,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()
Expand Down

0 comments on commit f14012e

Please sign in to comment.