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 code deprecated in 0.22 (released on October 13, 2022) [pulse] #10810

Closed
3 tasks
1ucian0 opened this issue Sep 11, 2023 · 9 comments · Fixed by #11024
Closed
3 tasks

Remove code deprecated in 0.22 (released on October 13, 2022) [pulse] #10810

1ucian0 opened this issue Sep 11, 2023 · 9 comments · Fixed by #11024
Assignees
Labels
good first issue Good for newcomers

Comments

@1ucian0
Copy link
Member

1ucian0 commented Sep 11, 2023

There is code released 9 month ago that needs to be removed.

  • qiskit/pulse/utils.py:113
  • qiskit/pulse/library/parametric_pulses.py:66
  • qiskit/pulse/library/parametric_pulses.py:135
  • qiskit/pulse/library/parametric_pulses.py:247
  • qiskit/pulse/library/parametric_pulses.py:417
  • qiskit/pulse/library/parametric_pulses.py:540

This includes:

  • Remove deprecated code and tests (if they only check the deprecation raise)
  • tag the PR as Changelog: Removal
  • create a release note in the category upgrade:. If you can include an example with an alternative for user to migrate to the new code, as this change might break users code.

Related:

@Raghav-Bell
Copy link
Contributor

Please assign this issue to me.

@1ucian0
Copy link
Member Author

1ucian0 commented Sep 12, 2023

Hi @Raghav-Bell , thanks for volunteer! If you don't mind, let's first close some of the open PRs and we can leave good first issue to new comers. What do you think?

@Raghav-Bell
Copy link
Contributor

Yes @1ucian0 I agree.

@Ak-ash22
Copy link
Contributor

If this issue still needs attention...Can I be assigned to this? I am new comer.. I wanna try this out. Thank you

@abhamra
Copy link

abhamra commented Oct 2, 2023

I am also interested in contributing as a newcomer, please let me know if this task can be assigned to me

@1ucian0 1ucian0 moved this from Tagged but unassigned to Assigned in Contributor Monitoring Oct 4, 2023
@github-project-automation github-project-automation bot moved this to Tagged but unassigned in Contributor Monitoring Oct 4, 2023
@1ucian0
Copy link
Member Author

1ucian0 commented Oct 4, 2023

@Ak-ash22, assigning you! Thanks!

@Ak-ash22
Copy link
Contributor

Ak-ash22 commented Oct 8, 2023

Hello @1ucian0 ...This is my first time contributing.. kindly help me verify if this is correct :
I have removed all the '@deprecated_func(additional_msg=(..........)) ' and also the line 51 'from qiskit.utils.deprecation import deprecate_func' from the parametric_pulses.py file.
Also utils.py file doesnt seem to have line 113 in it?. Am I missing something here?

@1ucian0
Copy link
Member Author

1ucian0 commented Oct 9, 2023

The line numbers in the original post might be out of date. I removed qiskit/pulse/utils.py:113 in #10881, so no worry about that one. The rest, sounds good.

@Ak-ash22
Copy link
Contributor

Ak-ash22 commented Oct 9, 2023

@1ucian0 I noticed that all the classes in the parametric_pulses.py have been deprecated, so I decided to delete the entire file. Also I noticed a bug in the related test.... in the qiskit/test/python/pulse/test_pulse_lib.py, there is a test class named TestParametricPulses but it calls all the Gaussian, Constant etc waveforms from the symbolic_pulses.py and not from the paramteric_pulses.py. So I changed the name of the test class to TestSymbolicPulses(..). Also I made sure that there is no test case for symbolic pulses, just to be sure I am not duplicating it.
Let me know if these changes are ok?! so that I can do tox tests and move forward with submitting a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants