Remove legacy qasm2 parser #11308
Remove legacy qasm2 parser #11308
Conversation
This commit removes the legacy qasm2 parser. This was the original qasm parser in qiskit dating back to the very beginning of the project, but it has been superseded in recent releases by the faster more correct/strict rust parser. We no longer need to keep around two parsers for qasm2. This commit removes the legacy one and it's associated functions (like the ast converter).
mtreinish
added
Changelog: Removal
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 7026617853Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
|
The open question for me here is what we want to do about the pygments formatter? (Which is the source of the lint failures). Do we remove the argument from the exporter or do we add openqasm-pygments to the optionals list. Personally I would be in favor of just removing the argument I think. |
|
The other choice about Pygments in the near term is to change it to look up |
|
I'll just drop |
This commit removes the QuantumCircuit.qasm() method as part of the legacy parser removal. The exporter method was tied to some of the internals in the now removed `qiskit.qasm` module. The qasm() method was already queued up for eventual removal, this commit just pushes that forward so we don't have to add more comptaibility code into it to keep it working as is without the qiskit.qasm module existing anymore.
There was a problem hiding this comment.
Thanks for this - good to get this all cleaned up. We unified the terminology around OQ2 in #9351, so we probably should match that in the release note.
| def qasm( | ||
| self, | ||
| formatted: bool = False, | ||
| filename: str | None = None, | ||
| encoding: str | None = None, | ||
| ) -> str | None: |
There was a problem hiding this comment.
We need to remember to follow up this removal with a 0.46 deprecation if we're going ahead with it for 1.0 (which I think we should).
releasenotes/notes/remove-legacy-qasm2-parser-53ad3f1817fd68cc.yaml
Outdated
Show resolved
Hide resolved
| @@ -1,6 +1,5 @@ | |||
| rustworkx>=0.13.0 | |||
| numpy>=1.17,<2 | |||
| ply>=3.10 | |||
There was a problem hiding this comment.
Very pleased to see this going, especially since it's not been maintained as an actual Python package since 2020.
There was a problem hiding this comment.
This was actually my motivation for this PR, I was looking at the requirements list and forgot this was in there.
releasenotes/notes/remove-legacy-qasm2-parser-53ad3f1817fd68cc.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Jake Lishman <jake@binhbar.com>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
The removal broke Randomized tests #2645 (comment) . Probably an easy fix. |
|
Yeah, the randomised tests just need to switch to using |
The legacy qasm2 parser was remvoed on the main branch already in Qiskit#11308 for the 1.0.0 release. To ensure that users of the 0.x release series are warned of this API change this commit marks the functionality removed in Qiskit#11308 as deprecated for the 0.46.0 release.
* Deprecate legacy qasm2 parser for 0.46 The legacy qasm2 parser was remvoed on the main branch already in #11308 for the 1.0.0 release. To ensure that users of the 0.x release series are warned of this API change this commit marks the functionality removed in #11308 as deprecated for the 0.46.0 release. * Update qiskit/converters/ast_to_dag.py --------- Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Summary
This commit removes the legacy qasm2 parser. This was the original qasm parser in qiskit dating back to the very beginning of the project, but it has been superseded in recent releases by the faster more correct/strict rust parser. We no longer need to keep around two parsers for qasm2. This commit removes the legacy one and it's associated functions (like the ast converter).
Details and comments