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

Update top level provider documentation for BackendV2 #7323

Merged
merged 5 commits into from
Dec 1, 2021

Conversation

mtreinish
Copy link
Member

Summary

As a follow-on to #5885 this commit updates the top level
qiskit.providers module documentation for BackendV2. This primarily
consists of two things, updating the writing a provider section to use
BackendV2 instead of BackendV1 and adding a migration section on user
facing changes between BackendV1 and BackendV2.

Details and comments

@mtreinish mtreinish added on hold Can not fix yet documentation Something is not clear or an error documentation priority: high Changelog: None Do not include in changelog labels Nov 29, 2021
@mtreinish mtreinish added this to the 0.19 milestone Nov 29, 2021
@mtreinish
Copy link
Member Author

mtreinish commented Nov 29, 2021

This PR is based on #5885's branch and is on hold until #5885 merges. After #5885 merges I'll rebase this on main and remove the on hold label. To see the changes in this PR you can look at the last commit: 4e4b4d1

@coveralls
Copy link

coveralls commented Nov 29, 2021

Pull Request Test Coverage Report for Build 1527872187

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 82.963%

Totals Coverage Status
Change from base Build 1527703798: 0.008%
Covered Lines: 50767
Relevant Lines: 61192

💛 - Coveralls

@mtreinish mtreinish mentioned this pull request Nov 29, 2021
8 tasks
@mtreinish mtreinish changed the title [WIP] Update top level provider documentation for BackendV2 Update top level provider documentation for BackendV2 Nov 29, 2021
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Just a quick read through. I don't know enough about what's happened in the main BackendV2 PR to properly review the content, but the bits I do know looked correct to me.

qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/__init__.py Show resolved Hide resolved
@mtreinish mtreinish force-pushed the backend-v2-toplevel-docs branch 3 times, most recently from 12673dd to b445912 Compare November 30, 2021 15:00
As a follow-on to Qiskit#5885 this commit updates the top level
qiskit.providers module documentation for BackendV2. This primarily
consists of two things, updating the writing a provider section to use
BackendV2 instead of BackendV1 and adding a migration section on user
facing changes between BackendV1 and BackendV2.
@mtreinish mtreinish removed the on hold Can not fix yet label Dec 1, 2021
@mtreinish
Copy link
Member Author

Ok, now that #5885 has merged I've rebased this on the merged version and it's no longer blocked

@1ucian0
Copy link
Member

1ucian0 commented Dec 1, 2021

The following sentence sounds repetitive:

For a simple example of a provider the
`qiskit-aqt-provider <https://github.com/qiskit-community/qiskit-aqt-provider>`__
package is worth using as an example.

I suggest:

For a simple example of a provider, see the
`qiskit-aqt-provider <https://github.com/qiskit-community/qiskit-aqt-provider>`__
package.

Comment on lines +136 to 139
:meth:`~~qiskit.providers.BackendV2.run` method.

* Add any custom gates for the backend's basis to the session
:class:`~qiskit.circuit.EquivalenceLibrary` instance.
Copy link
Member

Choose a reason for hiding this comment

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

Is this spacing intentional?

Screenshot 2021-12-01 at 21 44 55

Suggested change
:meth:`~~qiskit.providers.BackendV2.run` method.
* Add any custom gates for the backend's basis to the session
:class:`~qiskit.circuit.EquivalenceLibrary` instance.
:meth:`~~qiskit.providers.BackendV2.run` method.
* Add any custom gates for the backend's basis to the session
:class:`~qiskit.circuit.EquivalenceLibrary` instance.

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 is, the custom gates thing is a sub-bullet of building a backend (as it's an optionally a part of building a backend class) see where @jakelishman asked the same thing: #7323 (comment)

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
@mtreinish
Copy link
Member Author

The following sentence sounds repetitive:

For a simple example of a provider the
`qiskit-aqt-provider <https://github.com/qiskit-community/qiskit-aqt-provider>`__
package is worth using as an example.

I suggest:

For a simple example of a provider, see the
`qiskit-aqt-provider <https://github.com/qiskit-community/qiskit-aqt-provider>`__
package.

Done in: 254270d I also updated the link to the repo's current home

1ucian0
1ucian0 previously approved these changes Dec 1, 2021
Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mergify mergify bot merged commit b68b712 into Qiskit:main Dec 1, 2021
@mtreinish mtreinish deleted the backend-v2-toplevel-docs branch December 2, 2021 18:28
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.

4 participants