Qiskit / qiskit-tutorials

A collection of Jupyter notebooks showing how to use the Qiskit SDK
Apache License 2.0
2.28k stars 1.28k forks source link

Revert "Fix catch in grover tutorial for tera 0.24.0 compatibility (#1449)" #1466

Closed Eric-Arellano closed 1 year ago

Eric-Arellano commented 1 year ago

Now that Terra 0.24.1 has been released, we can revert this change.

This doesn't actually impact what we deploy to https://qiskit.org/documentation/tutorials/algorithms/07_grover_examples.html, which depends on the metapackage repository: https://github.com/Qiskit/qiskit-metapackage/pull/1757. But it's good to have this notebook showing something more useful.

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

frankharkins commented 1 year ago

Would it break anything to just remove the try/except blocks completely? They allow the notebook to "run" if tweedledum isn't installed, but it seems pointless as the notebook produces no output.

Eric-Arellano commented 1 year ago

In general, this tutorial is a challenge because it means that we can't build docs on Apple Silicon, which is as of yesterday the sole type of Mac now being sold.

Do we know how relevant this tutorial still is? Is it possible to rewrite it or delete it?

frankharkins commented 1 year ago

Do you just need to build on M1 for local testing? If so, you might be able to use the raises-exception tag instead of the try/except blocks https://nbsphinx.readthedocs.io/en/0.9.2/allow-errors.html

Eric-Arellano commented 1 year ago

Do you just need to build on M1 for local testing? If so, you might be able to use the raises-exception tag instead of the try/except blocks https://nbsphinx.readthedocs.io/en/0.9.2/allow-errors.html

Yeah, it's important that devs can build docs locally. It encourages them to check the quality of docs more. That was a project when I first joined to get tox -e docs working.

Rather than raises-exception, I think the best move would be improving the try/except to instead assert our specific conditions: if it's Python < 3.11 and not macOS, then eagerly error. Else, silently do nothing. That is because Tweedledum can't handle those situations. We started discussing in the Qiskit dev meeting this week what the long term plan for tweedledum is.

frankharkins commented 1 year ago

Ok yeah that makes sense, this has more parts to it than I realised. Don't let this block merging the PR.