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

Fix deprecated behaviour in timeline drawer #10846

Merged
merged 1 commit into from Oct 10, 2023

Conversation

jakelishman
Copy link
Member

@jakelishman jakelishman commented Sep 15, 2023

Summary

This removes the year-old deprecation of attempting to use the timeline drawer to draw an unscheduled circuit.

Internally, it removes all use of the deprecated Bit properties Bit.index and Bit.register. This is achieved by making it possible for generator functions to accept the complete QuantumCircuit program as a keyword argument, if they advertise that they can handle this by setting a special accepts_program attribute on themselves to True. This somewhat convoluted setup is because the generator functions are supposed to be arbitrary and user-definable in custom stylesheets, so we cannot unilaterally change the call signature.

Details and comments

Along with #10844, I believe this removes the last uses of Bit.index and Bit.register from the Terra code base. I'll follow up this (and that) PR with another that removes that warning skip from the test class (and any other warning skips that can be removed as well).

I had to add a new "feature" to the timeline object generators to do this in a backwards-compatible manner, since the data model the generators were working from is just fundamentally incorrect in current Qiskit, and there has been no way to produce the exactly correct results ever since #5486.

This PR originally removed the long-deprecated code that scheduled unscheduled circuits on entry to the canvas program loader. However, that deprecation was done incorrectly and was currently not being shown to users, so it actually needs to go through a proper deprecation cycle. The deprecated behaviour is also being used in the DynamicalDecoupling pass's docstring.

@jakelishman jakelishman added Changelog: New Feature Include in the "Added" section of the changelog Changelog: Removal Include in the Removed section of the changelog mod: visualization qiskit.visualization labels Sep 15, 2023
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

@coveralls
Copy link

coveralls commented Sep 15, 2023

Pull Request Test Coverage Report for Build 6207547275

  • 21 of 33 (63.64%) changed or added relevant lines in 2 files are covered.
  • 15 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.02%) to 87.25%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/visualization/timeline/core.py 7 12 58.33%
qiskit/visualization/timeline/generators.py 14 21 66.67%
Files with Coverage Reduction New Missed Lines %
qiskit/visualization/timeline/core.py 1 85.51%
crates/qasm2/src/lex.rs 2 91.16%
crates/qasm2/src/parse.rs 12 96.67%
Totals Coverage Status
Change from base Build 6204306855: -0.02%
Covered Lines: 74188
Relevant Lines: 85029

💛 - Coveralls

Internally, it removes all use of the deprecated `Bit` properties
`Bit.index` and `Bit.register`.  This is achieved by making it possible
for generator functions to accept the complete `QuantumCircuit` program
as a keyword argument, if they advertise that they can handle this by
setting a special `accepts_program` attribute on themselves to `True`.
This somewhat convoluted setup is because the generator functions are
supposed to be arbitrary and user-definable in custom stylesheets, so
we cannot unilaterally change the call signature.
@jakelishman jakelishman removed the Changelog: Removal Include in the Removed section of the changelog label Sep 16, 2023
@jakelishman jakelishman added this to the 0.45.0 milestone Sep 22, 2023
Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

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

Thanks Jake!

@1ucian0 1ucian0 added this pull request to the merge queue Oct 10, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 10, 2023
@jakelishman jakelishman added this pull request to the merge queue Oct 10, 2023
Merged via the queue into Qiskit:main with commit 2b2b079 Oct 10, 2023
14 checks passed
@jakelishman jakelishman deleted the fix-timeline-deprecated-bit branch October 10, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog mod: visualization qiskit.visualization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants