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

Document versioning and deprecation policy for major versions #11205

Closed
wants to merge 9 commits into from

Conversation

mtreinish
Copy link
Member

Summary

This commit document expands the versioning, deprecation, and stability polices to document the policy around major versions. The previous policies were all written without any consideration for major versioning. This outlines the minimum support windows for major versions and how deprecations and breaking changes will work with regards to versioning. It also explicitly documents that Qiskit is using semantic versioning. This is a actually a change from historical releases prior to Qiskit 0.44.0 where we explicitly did not follow semantic versioning as the version number was based on the qiskit elements.

This is an expanded document outlining the procedures decided on in the discussion around Qiskit/RFCs#34.

Details and comments

This commit document expands the versioning, deprecation, and stability
polices to document the policy around major versions. The previous
policies were all written without any consideration for major
versioning. This outlines the minimum support windows for major versions
and how deprecations and breaking changes will work with regards to
versioning. It also explicitly documents that Qiskit is using semantic
versioning. This is a actually a change from historical releases prior
to Qiskit 0.44.0 where we explicitly did not follow semantic versioning
as the version number was based on the qiskit elements.

This is an expanded document outlining the procedures decided on in
the discussion around Qiskit/RFCs#34.
@mtreinish mtreinish added documentation Something is not clear or an error documentation priority: high Changelog: None Do not include in changelog labels Nov 6, 2023
@mtreinish mtreinish added this to the 1.0.0pre1 milestone Nov 6, 2023
@mtreinish mtreinish requested a review from a team as a code owner November 6, 2023 19:35
@qiskit-bot
Copy link
Collaborator

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

@coveralls
Copy link

coveralls commented Nov 6, 2023

Pull Request Test Coverage Report for Build 6882844407

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 493 unchanged lines in 39 files lost coverage.
  • Overall coverage decreased (-1.0%) to 85.905%

Files with Coverage Reduction New Missed Lines %
qiskit/circuit/library/blueprintcircuit.py 1 92.24%
qiskit/circuit/library/evolved_operator_ansatz.py 1 85.12%
qiskit/opflow/state_fns/operator_state_fn.py 1 80.22%
qiskit/opflow/state_fns/state_fn.py 1 81.88%
qiskit/primitives/backend_sampler.py 1 98.84%
qiskit/primitives/utils.py 1 87.8%
qiskit/providers/backend.py 1 81.76%
qiskit/providers/basicaer/basicaerprovider.py 1 94.34%
qiskit/quantum_info/operators/symplectic/pauli_list.py 1 83.16%
qiskit/quantum_info/operators/symplectic/pauli.py 1 83.47%
Totals Coverage Status
Change from base Build 6827950580: -1.0%
Covered Lines: 65944
Relevant Lines: 76764

💛 - Coveralls

docs/deprecation_policy.rst Outdated Show resolved Hide resolved
mtreinish and others added 2 commits November 7, 2023 05:56
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
docs/deprecation_policy.rst Outdated Show resolved Hide resolved
docs/deprecation_policy.rst Outdated Show resolved Hide resolved
docs/deprecation_policy.rst Show resolved Hide resolved
docs/deprecation_policy.rst Outdated Show resolved Hide resolved
docs/deprecation_policy.rst Outdated Show resolved Hide resolved
docs/deprecation_policy.rst Outdated Show resolved Hide resolved
- we must not remove or change code without active warnings for least three
months or two complete version cycles;
- we must not remove or change code without active warnings on a supported
release series for at least three months and removals can only occur on
Copy link
Contributor

Choose a reason for hiding this comment

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

The three month window is a little unclear to me. Regarding removals, couldn't it just be said that removals should only be done in the next major release, without regard to a window? And should changes be allowed with a three month warning? Or should they just be kicked to the next major release as well?

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'll just remove this. I left it in because in the semantic versioning model everything will be deprecated for at least 6 months because of the extended support. I was thinking we still wanted to have a minimum deprecation window duration, but in the model there isn't really path for it to be < 6 months so this isn't needed anymore.

on that release series.

For the purposes of semantic versioning, the Qiskit public API is considered
any documented module, class, function, or method that is not marked as private
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the use of "documented" here, but I am not sure it agrees with the text in the "Deprecation Policy" section below:

Beware that users will often be using functions, classes and methods that we,
the Qiskit developers, may consider internal or not widely used.  Do not make
assumptions that "this is buried, so nobody will be using it"; if it is public,
it is subject to the policy.

and

never assume that a function that isn't explicitly internal isn't in use

If something is not documented but does not start with a _, can it be removed? Should "public" in the quoted section above be "publicly documented?"

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 should update the deprecation policy section below I think. There was an ambiguity in expectations for some people around public functions that are in weird modules. For example something like qiskit.transpiler.passes.layout.vf2_utils.score_layout() which is not documented in the published api docs and I wrote to be internal but under the deprecation policy wording would be viewed as public. My thinking by explicitly saying documented is to tighten the potential surface and make it clear what's expected to be treated as public.

I'll reword this a bit to be more clear, because I think there are some cases where private methods are part of a stable interface. The best example I can think of are things like interface definitions that have abstract private methods that need to be defined.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, there are a couple of cases where _* names are public:

  • Advanced-usage low-level methods we commit to stability in, for downstream code to use if they guarantee to uphold the class/safety invariants themselves. The archetypal example of this is QuantumCircuit._append, which is meant to be documented, but I think Sphinx might be overlooking it at the moment.
  • Abstract methods in base classes that are explicitly meant for subclasses to override. These are often prefixed with an underscore to show that users aren't meant to call them directly, but the interface classes call them. Things like SingletonInstruction._singleton_lookup_key fall into this category.

for at least 6 months; only bug and security fixes will be accepted during this
time and only patch releases will be published for this major version. A final
patch version will be published when support is dropped and that release will
also document the end of support for that major version series. A longer
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it mean that it documents the end of the release? Its release notes will say that there is no more support? If there are no bug fixes at that time, will it just be identical to the previous patch release other than the release notes?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just the release notes. I was thinking we'd just put in the prelude something like: "0.46.5 is the final release of the 0.x release series of Qiskit. There will be no future releases of 0.x and you should migrate to using a 1.x release."

version. Immediately preceding a new major version a final minor version will
be published. This final minor version release ``0.N+1.0`` is equivalent to
``0.N.0`` but with warnings and deprecations for any API changes that are
made on the new major version series.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the "deprecation policy" section below might need to be updated to account for this final minor version and the new use of major versions. It talks about removing features only after providing an alternate path for one minor, warnings for one minor, and then removal at least two minors after the warnings. I think the idea is that with this final minor Qiskit can issue warnings for things removed in the next major even though they are released almost simultaneously because the final minor has an extended support window? I am not sure about the requirement to provide an alternate path before removal under the new semantic versioning model. If removing a feature in Qiskit 2.0, does anything have to happen in Qiskit 1 besides a deprecation in the final minor version?

Copy link
Member Author

@mtreinish mtreinish Nov 7, 2023

Choose a reason for hiding this comment

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

I was thinking that ideally we'd still be adding the warnings and the alternative features in minor version releases ahead of the the 1->2 migration. In those cases we'd still want to follow that guidance. But in general yeah you're right there isn't a hard requirement for that, especially at the 1->2 migration point. I'll update the deprecation section to take this into account.

Co-authored-by: Will Shanks <wshaos@posteo.net>
like normal.
open a PR to the most recent supported stable branch. You can review and merge
this PR like normal. If you're backporting from main to a different stable
branch you can leave a comment of the form ``@Mergifyio backport branch`` on a
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worth adding a previous stable backport potential label to .mergify.yml?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yeah this is a good idea we can totally do this. I'll update the docs to include this.

Copy link
Member

Choose a reason for hiding this comment

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

If we're going this route, I wouldn't mind doing it as explicit labels (backport to 0.45, backport to 1.1, etc) - it's more work to manage Mergify since we can't easily make it dynamic on the label, but I already sometimes forget what "stable backport potential" means when we're in an rc period.

docs/deprecation_policy.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 left a comment

Choose a reason for hiding this comment

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

Thanks Matthew. Overall what this document describes looks reasonable and gives useful guideline for community developers.

My main concern is the pattern to develop a new feature. Since Qiskit doesn't use feature branches, this document implicitly suggests developers to use versioned classes to buffer the API change. This might be manageable if the package exposes only several public classes. However if every class gains its version, the class dependency would become too complicated to deal with, e.g. QuantumCircuitV2 supports GateV4 inherit from InstructionV3, and ScheduleBlockV5 for calibration. Then we would want to alias a class at package level. For example import QuantumCircuitV2 as QuantumCircuit which is breaking API change to users. It doesn't need to reside in this document, but we may want some reasonable pattern for new feature development before accepting this release policy. Probably we can make the most use of RFC?


For example, on the release of Qiskit 1.0.0 a 0.46.0 release was published
immediately proceeding the 1.0.0 release. The 0.46.0 release was equivalent
to the 0.45.0 release but with additional deprecation warnings that document
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe 0.45.x since 46 is built upon the latest patch release.

Another question would be; as a developer in which version can we add deprecation warnings if we want to upgrade API in next major release? Do we need to wait for N.M+1.0 and open a separate PR to that branch for deprecation? Seems like we need to keep it in our todo list for a year at most.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can add deprecations in any minor/major version, the exception being 1.0, because we're trying to make 1.0 deprecation warning free. For example I'm already planning deprecations for 1.1: #11212

docs/deprecation_policy.rst Outdated Show resolved Hide resolved
on that release series.

For the purposes of semantic versioning, the Qiskit public API is considered
any documented module, class, function, or method that is not marked as private
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, there are a couple of cases where _* names are public:

  • Advanced-usage low-level methods we commit to stability in, for downstream code to use if they guarantee to uphold the class/safety invariants themselves. The archetypal example of this is QuantumCircuit._append, which is meant to be documented, but I think Sphinx might be overlooking it at the moment.
  • Abstract methods in base classes that are explicitly meant for subclasses to override. These are often prefixed with an underscore to show that users aren't meant to call them directly, but the interface classes call them. Things like SingletonInstruction._singleton_lookup_key fall into this category.

like normal.
open a PR to the most recent supported stable branch. You can review and merge
this PR like normal. If you're backporting from main to a different stable
branch you can leave a comment of the form ``@Mergifyio backport branch`` on a
Copy link
Member

Choose a reason for hiding this comment

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

If we're going this route, I wouldn't mind doing it as explicit labels (backport to 0.45, backport to 1.1, etc) - it's more work to manage Mergify since we can't easily make it dynamic on the label, but I already sometimes forget what "stable backport potential" means when we're in an rc period.

Co-authored-by: Jake Lishman <jake@binhbar.com>
mtreinish added a commit to mtreinish/qiskit-documentation that referenced this pull request Nov 16, 2023
This commit adds a new document to the hosted documentation for
Qiskit's versioning and stability policy. This is a continuation of
Qiskit/qiskit#11205 which documents for Qiskit users the versioning
and stability policy for the library. It includes an explanation of
how to interpret version numbers, the tentative release schedule for
the project, as well as a definition of what interfaces are considered
stable. This outlines the minimum support windows for major versions
and how deprecations and breaking changes will work with regards to
versioning. This documentation is critical to include with the Qiskit
1.0.0 release as it establishes the expectations around the major
version release for users (all previous documentation on the subject
tactically avoided the discussion of a major version).
@mtreinish
Copy link
Member Author

I'm going to close this PR, because of the change in the documentation strategy I've reopened this here: Qiskit/documentation#366 which is where it'll get documented from. There are still going to be the changes needed to the developer documentation around deprecations. Those will live in markdown files that aren't included in the doc builds by default here: #11218. I'll rework that piece of this PR into a new PR after #11218 merges

@mtreinish mtreinish closed this Nov 16, 2023
github-merge-queue bot pushed a commit to Qiskit/documentation that referenced this pull request Dec 1, 2023
This commit adds a new document to the hosted documentation for Qiskit's
versioning and stability policy. This is a continuation of
Qiskit/qiskit#11205 which documents for Qiskit users the versioning and
stability policy for the library. It includes an explanation of how to
interpret version numbers, the tentative release schedule for the
project, as well as a definition of what interfaces are considered
stable. This outlines the minimum support windows for major versions and
how deprecations and breaking changes will work with regards to
versioning. This documentation is critical to include with the Qiskit
1.0.0 release as it establishes the expectations around the major
version release for users (all previous documentation on the subject
tactically avoided the discussion of a major version).

---------

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: abbycross <across@us.ibm.com>
Co-authored-by: Kevin Hartman <kevin@hart.mn>
Co-authored-by: javabster <abby.s.mitchell@gmail.com>
Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog documentation Something is not clear or an error documentation priority: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants