Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated functionality from extensions, qasm, and dagcircuit modules #7208

Merged
merged 3 commits into from
Nov 5, 2021

Conversation

mtreinish
Copy link
Member

Summary

This commit goes through qiskit.extensions, qiskit.qasm, qiskit.util,
and qiskit.dagcircuit modules and cleans up deprecated functionality
that has gone through a full deprecation cycle and no longer needs to
be supported.

Details and comments

This commit goes through qiskit.extensions, qiskit.qasm, qiskit.util,
and qiskit.dagcircuit modules and cleans up deprecated functionality
that has gone through a full deprecation cycle and no longer needs to
be supported.
@mtreinish mtreinish added the Changelog: Removal Include in the Removed section of the changelog label Nov 1, 2021
@mtreinish mtreinish changed the title Remove deprecated functionality Remove deprecated functionality from extensions, qasm, util, and dagcircuit modules Nov 1, 2021
@jakelishman
Copy link
Member

We may need to leave qiskit.util another cycle, but issue a proper deprecation warning out of it. Of what I've got cloned, Aer and the old IBMQ still reference the old form, and looking at qiskit/util.py, it's not actually issuing any deprecation warnings, so people may not be aware. (See the two PRs linked above.)

For linking purposes: Ali also has #7011 to clean up qiskit.circuit.

@mtreinish
Copy link
Member Author

Hmm, yeah I'll back it out the qiskit.util removal from this pr and do it later. We couldn't add deprecation warnings because the only way I could think of doing it besides add a wrapper function for each function in the module was to use a module getattr function. But that was added in python 3.7 so it wouldn't work on 3.6.

@jakelishman
Copy link
Member

Why not just issue the deprecation warning raw in the util.py file? As long as qiskit/__init__.py doesn't import it, it won't get triggered until a user does the import, and modern Terra doesn't import it.

@jakelishman
Copy link
Member

I mean something like having the file be:

import warnings
from qiskit.utils import *

warnings.warn("Use qiskit.utils instead", DeprecationWarning)

It isn't imported by Terra during init, so users won't see the warning until they import it themselves, and you can't use any functions through its namespace without importing it. The warning can be attached to the import, rather than the member access, I think?

The qiskit/util.py will need to be deprecated a bit longer it's still in
active use by downstream packages and it was right on the boundary of
being deprecated long enough anyway so being a bit more conservative on
this is probably for the best anyway.
@mtreinish mtreinish force-pushed the remove-deprecated-stuff branch from c3a725d to d43072b Compare November 2, 2021 11:39
@mtreinish
Copy link
Member Author

I did try that when I first pushed up #5420 but back then qiskit.util was getting imported by things in the root of the package (probably from aer or ibmq which at the time got imported if available for qiskit.Aer and qiskit.IBMQ). But, yeah we can do that now as things have shifted around it should work fine. Do you want to push up a PR adding the warning?

@mtreinish mtreinish changed the title Remove deprecated functionality from extensions, qasm, util, and dagcircuit modules Remove deprecated functionality from extensions, qasm, and dagcircuit modules Nov 2, 2021
@jakelishman
Copy link
Member

Sure - I'll do it in a bit. I want to go through and check the other downstream qiskit packages first for uses of qiskit.util, and see if there's a sensible way we can put in the deprecation warning without heavily leaking that through to users; the functions in qiskit.utils are often the type that get actually run during module initialisation, so there's a good chance even importing Aer/whatever will show something to users (though perhaps #7204 will help with that).

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These deprecations look good to me now.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1425762814

  • 38 of 39 (97.44%) changed or added relevant lines in 29 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 82.496%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/tools/jupyter/version_table.py 0 1 0.0%
Totals Coverage Status
Change from base Build 1424775756: 0.1%
Covered Lines: 49618
Relevant Lines: 60146

💛 - Coveralls

@mergify mergify bot merged commit 6d88dd0 into Qiskit:main Nov 5, 2021
@mtreinish mtreinish deleted the remove-deprecated-stuff branch November 8, 2021 20:34
@jakelishman jakelishman mentioned this pull request Nov 10, 2021
@kdk kdk added this to the 0.19 milestone Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Removal Include in the Removed section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants