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 algorithms tutorials #10682

Merged
merged 5 commits into from Aug 25, 2023

Conversation

Eric-Arellano
Copy link
Collaborator

The algorithm tutorials now live in https://github.com/qiskit-community/qiskit-algorithms as of qiskit-community/qiskit-algorithms#44.

We (slightly prematurely) already set up redirects from qiskit.org/documentation/tutorials/algorithms/* to qiskit.org/ecosystem/algorithms/tutorials/index.html. So there is no reason to keep the tutorials in Qiskit anymore.

This is prework for copying over the qiskit-tutorials repository because we will not copy over the algorithms files to this repo.

(slightly prematurely)

We deployed the redirects before the new tutorials are actually live on the Ecosystem site due to bad coordination. My bad.

@Eric-Arellano Eric-Arellano added documentation Something is not clear or an error documentation stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: None Do not include in changelog labels Aug 21, 2023
@qiskit-bot
Copy link
Collaborator

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

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5940217275

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.001%) to 87.247%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 2 90.38%
Totals Coverage Status
Change from base Build 5939223424: 0.001%
Covered Lines: 74313
Relevant Lines: 85175

💛 - Coveralls

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.

I think in principle this is ok, though it still feels a bit wrong to merge it before the alternatives are published (even if the redirects are currently borking the links - that's kind of a separate problem).

Would it be appropriate to put a little note at the bottom of the index page saying

Looking for tutorials on algorithms? They've moved to a separate package here [...]

or something like that?

@ElePT
Copy link
Contributor

ElePT commented Aug 22, 2023

FYI, the qiskit_algorithms docs deployment is blocked by the 0.2.0 release, which only needs 1 more PR to be merged to be ready, so it should probably be possible to do tomorrow.

@Eric-Arellano
Copy link
Collaborator Author

Would it be appropriate to put a little note at the bottom of the index page saying

Good idea! I also added a warning about the operators tutorials. Once we copy over the tutorials to live in Terra, I want to add a warning to the top of those tutorials too.

I think in principle this is ok, though it still feels a bit wrong to merge it before the alternatives are published (even if the redirects are currently borking the links - that's kind of a separate problem).

I think it's a related problem. There's no reason to have the tutorials currently because the redirects botch it. We're one day away from the live release. Also, there's a slow turnaround for changes to our Cloudflare config, which is why the redirects got set up prematurely in the first place to try to get ahead of that. So, it would not make sense to try to remove the bad redirects today, then add them back tomorrow.

The Simulators tutorials have moved to
`Qiskit Aer <https://qiskit.org/ecosystem/aer/tutorials/index.html>`_
and the Algorithms tutorials to
`Qiskit Algorithms <https://qiskit.org/ecosystem/algorithms/tutorials/index.html>`_.
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to say a little more here? Not only have they moved, to take advantage of them you also need to refactor any code you have over from qiskit.algorithms to qiskit_algorithms. There is some text in the Qiskit release notes around that - this does not link to it exactly but its in the prelude https://qiskit.org/documentation/release_notes.html#release-notes-0-25-0-prelude Also qiskit_algorithms release note says something similar https://qiskit.org/ecosystem/algorithms/release_notes.html#release-notes-0-1-0 if thats preferable.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a warning that would better fit the documentation on the qiskit_algorithms side, we could add a note there warning about the change in the tutorials with respect to qiskit.

@Eric-Arellano
Copy link
Collaborator Author

FYI the tutorials are now live: https://qiskit.org/ecosystem/algorithms/tutorials/index.html

Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

LGTM, the algorithms docs are finally deployed and I think that we can take care of the refactoring warning Steve mentioned on the qiskit_algorithms side.

@jakelishman jakelishman added this pull request to the merge queue Aug 25, 2023
Merged via the queue into Qiskit:main with commit 021b0ff Aug 25, 2023
14 checks passed
mergify bot pushed a commit that referenced this pull request Aug 25, 2023
* Remove algorithms tutorials

* Also update github

* Add guidance on tutorials index page

* Bad version equivalency

(cherry picked from commit 021b0ff)
@Eric-Arellano Eric-Arellano deleted the rm-algorithms-tutorials branch August 25, 2023 17:08
github-merge-queue bot pushed a commit that referenced this pull request Aug 25, 2023
* Remove algorithms tutorials

* Also update github

* Add guidance on tutorials index page

* Bad version equivalency

(cherry picked from commit 021b0ff)

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@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 stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants