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

Rename show_idle and show_barrier to idle_wires and plot_barriers #11878

Merged
merged 6 commits into from
Apr 24, 2024

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Feb 23, 2024

Fixes #11858

The parameters show_idle and show_barrier in the timeline drawers had been replaced by idle_wires and plot_barriers respectively to match the circuit drawer parameters. Their previous names are now deprecated and will be removed in the next major release. The new parameters are fully equivalent.

@1ucian0 1ucian0 added the Changelog: API Change Include in the "Changed" section of the changelog label Feb 23, 2024
@1ucian0 1ucian0 added this to the 1.1.0 milestone Feb 23, 2024
@qiskit-bot
Copy link
Collaborator

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

@coveralls
Copy link

coveralls commented Feb 23, 2024

Pull Request Test Coverage Report for Build 8109859461

Details

  • 7 of 16 (43.75%) changed or added relevant lines in 3 files are covered.
  • 10 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.02%) to 89.296%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/pulse/schedule.py 2 3 66.67%
qiskit/visualization/pulse_v2/interface.py 2 4 50.0%
qiskit/visualization/timeline/interface.py 3 9 33.33%
Files with Coverage Reduction New Missed Lines %
qiskit/pulse/schedule.py 1 87.5%
crates/qasm2/src/lex.rs 3 91.94%
crates/qasm2/src/parse.rs 6 97.15%
Totals Coverage Status
Change from base Build 8104544265: 0.02%
Covered Lines: 59131
Relevant Lines: 66219

💛 - Coveralls

Comment on lines +39 to +40
idle_wires: Optional[bool] = None,
plot_barriers: Optional[bool] = None,
Copy link
Member

Choose a reason for hiding this comment

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

Per the deprecation policy we need to add plot_barriers and idle_wires before we start emitting a deprecation warning here. This way users have a release of overlap where they can use the old and new without a deprecation warning being emitted. If deprecate_arg has a pending arg setting that would be enough, otherwise we'll have to remove the decorators and just add the new arguments.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added the pending=True in f4f6a64

qiskit/visualization/timeline/interface.py Outdated Show resolved Hide resolved
@mtreinish mtreinish added Changelog: New Feature Include in the "Added" section of the changelog and removed Changelog: API Change Include in the "Changed" section of the changelog labels Feb 23, 2024
---
deprecations:
- |
The parameters ``show_idle`` and ``show_barrier`` in the timeline drawers had been replaced by ``idle_wires`` and ``plot_barriers``
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we reword the reno to say that the new parameter names have been introduced and the old names will be deprecated in a following release?

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, we should either remove all mentions of deprecation timings in the release note and trust the user to know/read about our deprecation policy OR we should be accurate about when we deprecate and remove things...

That said, I think it is rather minor issue here and I will approve the changes. :-)

---
deprecations:
- |
The parameters ``show_idle`` and ``show_barrier`` in the timeline drawers had been replaced by ``idle_wires`` and ``plot_barriers``
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, we should either remove all mentions of deprecation timings in the release note and trust the user to know/read about our deprecation policy OR we should be accurate about when we deprecate and remove things...

That said, I think it is rather minor issue here and I will approve the changes. :-)

@1ucian0 1ucian0 added this pull request to the merge queue Apr 24, 2024
Merged via the queue into Qiskit:main with commit a827aaa Apr 24, 2024
12 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inconsistencies in the options for the drawer (circuit, and timeline)
6 participants