Skip to content

Addition of black-nb to CI check to avoid inconsistency in indentation #4020

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

Closed
wants to merge 18 commits into from
Closed

Addition of black-nb to CI check to avoid inconsistency in indentation #4020

wants to merge 18 commits into from

Conversation

paniash
Copy link

@paniash paniash commented Apr 13, 2021

Fixes #3976 and added quotations around variables. See here.

@paniash paniash requested review from cduck, vtomole and a team as code owners April 13, 2021 14:51
@google-cla
Copy link

google-cla bot commented Apr 13, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Apr 13, 2021
@google-cla
Copy link

google-cla bot commented Apr 13, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@paniash
Copy link
Author

paniash commented Apr 13, 2021

@googlebot I signed it!

@google-cla google-cla bot added cla: yes Makes googlebot stop complaining. and removed cla: no labels Apr 13, 2021
@vtomole
Copy link
Collaborator

vtomole commented Apr 13, 2021

@paniash Thanks for opening this. Why isn't the Notebook Formating check failing?

@paniash
Copy link
Author

paniash commented Apr 16, 2021

@vtomole Hmmm.... I'll have to check what's going wrong. Sorry about that. I'll do it over the weekend.

@vtomole
Copy link
Collaborator

vtomole commented Apr 27, 2021

@paniash The right test is failing but probably not for the right reason. The log says:

check/nbformat: line 51: [: missing `]'
check/nbformat: line 51: -z: command not found

@paniash
Copy link
Author

paniash commented Apr 27, 2021

@vtomole Hi! Thanks for letting me know. I've fixed it and looks good from my side. Let me know if it's alright. :-)

Copy link
Collaborator

@vtomole vtomole left a comment

Choose a reason for hiding this comment

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

I get

./check/nbformat: line 55: python3 -m tensorflow_docs.tools.nbfmt --indent=1: command not found

When I call ./check/nbformat --apply. This is at the end, after all the notebooks have been formatted.

Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

I ran this locally and had a bunch of issues:

check/nbformat: line 51: [: missing `]'
check/nbformat: line 51: -z: command not found
reformatted docs/qcvv/xeb_coherent_noise.ipynb
    14 cells reformatted, 2 cells left unchanged, 1 cell failed to reformat.
reformatted docs/circuits.ipynb
    5 cells reformatted, 12 cells left unchanged, 1 cell failed to reformat.
reformatted docs/protocols.ipynb
    4 cells reformatted, 5 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/quantum_walks.ipynb
    9 cells reformatted, 4 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/heatmaps.ipynb
    2 cells reformatted, 1 cell left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/google/floquet.ipynb
    16 cells reformatted, 17 cells left unchanged, 1 cell failed to reformat.
reformatted docs/qudits.ipynb
    2 cells reformatted, 1 cell failed to reformat.
reformatted docs/qcvv/isolated_xeb.ipynb
    10 cells reformatted, 4 cells left unchanged, 1 cell failed to reformat.
error: cannot format docs/custom_gates.ipynb: INTERNAL ERROR: Black produced different code on the second pass of the formatter.
reformatted docs/tutorials/educators/ion_device.ipynb
    3 cells reformatted, 4 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/qaoa.ipynb
    13 cells reformatted, 1 cell left unchanged, 1 cell failed to reformat.
reformatted docs/google/concepts.ipynb
    2 cells reformatted, 1 cell failed to reformat.
reformatted docs/tutorials/educators/qaoa_ising.ipynb
    13 cells reformatted, 10 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/hidden_linear_function.ipynb
    8 cells reformatted, 2 cells left unchanged, 1 cell failed to reformat.
reformatted docs/operators_and_observables.ipynb
    2 cells reformatted, 27 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/google/colab.ipynb
    4 cells reformatted, 1 cell failed to reformat.
reformatted docs/transform.ipynb
    3 cells reformatted, 1 cell failed to reformat.
error: cannot format docs/tutorials/educators/intro.ipynb: INTERNAL ERROR: Black produced different code on the second pass of the formatter.
reformatted docs/qubits.ipynb
    1 cell reformatted, 1 cell left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/basics.ipynb
    9 cells reformatted, 6 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/aqt/getting_started.ipynb
    6 cells reformatted, 1 cell left unchanged, 1 cell failed to reformat.
reformatted docs/simulation.ipynb
    12 cells reformatted, 4 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/google/echoes.ipynb
    7 cells reformatted, 7 cells left unchanged, 1 cell failed to reformat.
reformatted docs/noise.ipynb
    9 cells reformatted, 14 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/ionq/getting_started.ipynb
    6 cells reformatted, 3 cells left unchanged, 1 cell failed to reformat.
reformatted docs/interop.ipynb
    5 cells reformatted, 1 cell left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/google/visualizing_calibration_metrics.ipynb
    18 cells reformatted, 1 cell left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/google/reservations.ipynb
    10 cells reformatted, 1 cell failed to reformat.
reformatted docs/tutorials/shor.ipynb
    11 cells reformatted, 10 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/pasqal/getting_started.ipynb
    6 cells reformatted, 6 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/google/start.ipynb
    7 cells reformatted, 2 cells left unchanged, 1 cell failed to reformat.
reformatted docs/gates.ipynb
    2 cells reformatted, 1 cell failed to reformat.
reformatted docs/tutorials/educators/textbook_algorithms.ipynb
    12 cells reformatted, 20 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/rabi_oscillations.ipynb
    13 cells reformatted, 2 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/educators/chemistry.ipynb
    18 cells reformatted, 6 cells left unchanged, 1 cell failed to reformat.
reformatted docs/_template.ipynb
    1 cell reformatted, 1 cell failed to reformat.
reformatted docs/qcvv/parallel_xeb.ipynb
    13 cells reformatted, 5 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/educators/neutral_atom.ipynb
    7 cells reformatted, 14 cells left unchanged, 1 cell failed to reformat.
reformatted docs/tutorials/variational_algorithm.ipynb
    12 cells reformatted, 3 cells left unchanged, 1 cell failed to reformat.
reformatted docs/qcvv/xeb_theory.ipynb
    10 cells reformatted, 2 cells left unchanged, 1 cell failed to reformat.
All done! ✨ 🍰 ✨
38 files reformatted, 2 files failed to reformat.
check/nbformat: line 55: python3 -m tensorflow_docs.tools.nbfmt --indent=1: command not found

Thanks @paniash for working on this, though I'm not convinced yet that this is a good direction.

I think the first big question is: does black-nb and tf-docs.nbfmt collide with each other or not? By collide I mean that do they agree on the same formatting? If they don't agree, then we could never have a well formatted notebook!

Second main concern is that we need the tf_docs nbformat to be the last formatter, otherwise the site publishing will fail - they do run internally this nbfmt to ensure things are well formatted.

Third concern is that you haven't seem to have tested the user friendliness of the output in these cases:

  1. check/nbformat - should report all the unformatted notebooks by path. The script currently does not do that for the black unformatted notebooks.
The following notebooks require formatting.

Notebooks are not formatted. Please run 'check/nbformat --apply'
  1. check/nbformat --apply - there are a ton of issues where it fails on cells, is that okay to ignore or not? Does it report a non zero exit status? If it does, the Github Actions job will fail?

@vtomole
Copy link
Collaborator

vtomole commented Apr 30, 2021

Hey @paniash, Are you planning on opening a new PR with the changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Jupyter-black to check/format
3 participants