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 features from >3 months ago and TODOs #784

Closed
jyu00 opened this issue Mar 30, 2023 · 7 comments
Closed

Remove deprecated features from >3 months ago and TODOs #784

jyu00 opened this issue Mar 30, 2023 · 7 comments
Labels
enhancement New feature or request priority: medium Medium Priority issue (must have for current release)

Comments

@jyu00
Copy link
Collaborator

jyu00 commented Mar 30, 2023

What is the expected feature or enhancement?

Many features were deprecated in 0.7 release and can be removed now. In light of this issue, the TODOs in the code should be reviewed and actions taken accordingly.

Acceptance criteria

  • Remove features that had sufficient deprecation period per Qiskit's deprecation policy
  • Review/fix all TODOs
@jyu00 jyu00 added the enhancement New feature or request label Mar 30, 2023
@kt474 kt474 added this to the 0.9.3 milestone Apr 3, 2023
@kt474 kt474 modified the milestones: 0.9.3, 0.9.4 Apr 17, 2023
@kt474 kt474 modified the milestones: 0.9.4, 0.9.5 May 5, 2023
@merav-aharoni
Copy link
Contributor

@kt474 - deprecated code for which versions can be removed?

@kt474
Copy link
Member

kt474 commented May 11, 2023

@kt474 - deprecated code for which versions can be removed?

I believe all the deprecated code from 0.7 has been removed in #797 but there are plenty of TODO's in the codebase that can be addressed

@merav-aharoni
Copy link
Contributor

I looked at the deprecation message in session.py:

        if "backend" in options:
            issue_deprecation_msg(
                "'backend' is no longer a supported option within a session",
                "0.9",
                "Instead, specify a backend when creating a Session instance.",
                3,
            )

I tried to raise an error if backend is defined in options, instead of the deprecation message. This does not work, because session.run itself adds self._backend to its options. So if session.run is called more than once, it will have backend inside options.
What should the specification be here? I think that we should remove backend from options in all levels and add it as an extra parameter on all run levels, since in QiskitRuntimeService.run we extract it from options anyway. Having backend both as a class member and in options is confusing. @kt474 , @jyu00 - I'd appreciate your inputs.

@kt474
Copy link
Member

kt474 commented May 23, 2023

Yes, it is a bit confusing - originally, I believe we wanted all options passed into session.run() to be honored, but in terms of backend, it never worked because sessions don't support cross multiple backends. This deprecation message was meant to just inform users not to pass in a backend in the options and the doc strings say the same thing. I'm ok with just leaving this deprecation message for now, as I don't believe any users are doing this anymore anyway.

I agree that ideally, backend is not an option, but it would be a relatively big change unrelated to this original issue

@jyu00
Copy link
Collaborator Author

jyu00 commented May 24, 2023

We can just make a copy of the input options before adding backend to it. We really shouldn't be modifying a user's variable to begin with anyway.

We can remove/privatize both session.run() and service.run() once everything becomes primitives only (i.e. no more custom programs).

@kt474 kt474 added this to the 0.10.1 milestone May 30, 2023
@kt474 kt474 removed this from the 0.10.1 milestone Jun 9, 2023
@jyu00 jyu00 added the priority: high High priority issue (must have for current release) label Jun 12, 2023
@kt474 kt474 added this to the 0.11.1 milestone Jun 12, 2023
@kt474 kt474 removed this from the 0.11.1 milestone Jul 6, 2023
@kt474
Copy link
Member

kt474 commented Sep 27, 2023

I believe #1093 handles the last deprecation warning we can remove, after it's in we can close this issue

Update 11/08 : we've handled all deprecation warnings but there are still plenty TODOs it code base

@kt474 kt474 added this to the 0.13.0 milestone Sep 29, 2023
@kt474 kt474 removed this from the 0.13.0 milestone Oct 18, 2023
@drew-distefano drew-distefano added priority: medium Medium Priority issue (must have for current release) and removed priority: high High priority issue (must have for current release) labels Dec 1, 2023
@jyu00
Copy link
Collaborator Author

jyu00 commented Jun 19, 2024

closing this, but as part of the release process we should always check if there are deprecated features that can be removed.

@jyu00 jyu00 closed this as completed Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: medium Medium Priority issue (must have for current release)
Projects
None yet
Development

No branches or pull requests

4 participants