Deprecate PrimitivesV1 and their utils #11490
Conversation
Pull Request Test Coverage Report for Build 7456794166
💛 - Coveralls |
|
One or more of the the following people are requested to review this:
|
There was a problem hiding this comment.
I added a few comments that I think could help make the deprecation more straightforward for users, but I think that the most important thing here is to clearly understand the expected deprecation timeline. The deprecation policy is:
- introduce alternative to "old" code
- show deprecation warnings in "old" code
- (after period is over) remove "old" code
With a minimum period of 3 months or 2 releases (minor) in between steps.
I don't think a deprecation in 0.46 fits this flow, as the alternative (primitives V2) won't be introduced until 1.0. So a reasonable timeline would look like:
- introduce primitives V2 in qiskit 1.0
- show deprecation warnings for primitives V1 in a minor following 1.0 (not 100% sure, it might even need to be the next major, so correct me if I'm wrong)
- remove primitives V1 in qiskit 2.0 (earliest)
If we show deprecation warnings immediately, I think we might overload users with warnings and have a more negative effect than anything else. The idea is to give people some time to transition to the new interfaces.
| warn( | ||
| "The class ``BasePrimitiveResult`` is deprecated as of qiskit 0.46.0. " | ||
| "It will be removed no earlier than 3 months after the release date. " | ||
| "Use PrimitiveResult in PrimitiveV2 instead.", |
There was a problem hiding this comment.
I think it would help to show the path of the new PrimitiveResult class. From #11227 I take that would be:
| "Use PrimitiveResult in PrimitiveV2 instead.", | |
| "Use PrimitiveResult class in `qiskit.primitives.containers` instead.", |
| @@ -51,6 +47,7 @@ class Estimator(BaseEstimator[PrimitiveJob[EstimatorResult]]): | |||
| this option is ignored. | |||
| """ | |||
|
|
|||
| @deprecate_func(since="0.46.0", additional_msg="Use StatevectorEstimator instead.") | |||
There was a problem hiding this comment.
In the estimator V2 PR, it looks like the new class is also called Estimator, right? not StatevectorEstimator. But it can be told apart from the V1 estimator by the import path.
| @@ -52,6 +53,7 @@ class Sampler(BaseSampler[PrimitiveJob[SamplerResult]]): | |||
| option is ignored. | |||
| """ | |||
|
|
|||
| @deprecate_func(since="0.46.0", additional_msg="Use StatevectorSampler instead.") | |||
There was a problem hiding this comment.
Same comment as with the Estimator.
|
|
||
|
|
||
| @deprecate_func(since="0.46.0") |
There was a problem hiding this comment.
If the deprecation is with no replacement, we usually add a small message explaining it.
| --- | ||
| deprecations: | ||
| - | | ||
| PrimitivesV1 has been deprecated. Use PrimitivesV2 instead. |
There was a problem hiding this comment.
I don't know if people would understand what "PrimitivesV1" are, adding a list of what the deprecated classes are would help.
|
Since there are significant merge conflicts and it is not definite to deprecate these features at this time, I will close it once. |
|
Superseded by #12575 |
Summary
Deprecate
Details and comments