CI: Pre-commit to apply black/isort/flake8 on ipython notebooks

[cphyc]

PR Summary

This adds a pre-commit hook to automatically format with black our notebooks. This should respect our black/isort/pyupgrade configs.

[cphyc]

pre-commit.ci run

[neutrinoceros]

Depending on the size of the diff we'll get from this (likely, big), I'd advise that this would be postpone to after the incoming release.

[cphyc]

pre-commit.ci autofix

[cphyc]

@neutrinoceros I triggered the pre-commit.ci bot, but actually we could decide not to trigger it now and just activate the hook in this PR.
We could create an issue to have it triggered once 4.0 is released? Wdyt?

[neutrinoceros]

We could create an issue to have it triggered once 4.0 is released? Wdyt?

I'd put it in the 4.1 Milestone, drop the auto-fix commit for now, and switch this to draft for now

[cphyc]

Note: deactivating E402 (https://www.flake8rules.com/rules/E402.html) as it seems to clash with adding some line magics in cells.

[cphyc]

Todo:

• once we're happy with this PR, call pre-commit.ci autofix
• inspect whether we can reactivate E402 with flake or not

[MarcoGorelli]

Hey, thanks for trying out nbQA!

Just a note on one of the error messages:

doc/source/analyzing/mesh_filter.ipynb:cell_4:4:5: E999 SyntaxError: invalid syntax

This looks like a bug in nbQA - cell4, after black has been run, becomes

print("Temperature of all cells:\n ad['temperature'] = \n%s\n" % ad["temperature"])
print(
    "Temperatures of all \"hot\" cells:\n hot_ad['temperature'] = \n%s"
    % hot_ad["temperature"]
)

and that's definitely valid syntax...I'll come back to you on this, sorry about that, but thanks for helping to find this bug!

[MarcoGorelli]

Got it, it's because IPython.core.inputsplitter.IPythonInputSplitter thinks "% hot_ad["temperature"]" is IPython magic:

(Pdb) !line
'    % hot_ad["temperature"]\n'
(Pdb) IPythonInputSplitter().transform_cell(line)
'get_ipython().run_line_magic(\'hot_ad\', \'["temperature"]\')\n'

I think the solution is for nbQA to simply stop trying to handle inline magics, it's just too complicated / unreliable and leads to bugs like this one. I'll ping when a new version's out, hopefully this weekend!

[neutrinoceros]

Thanks a lot @MarcoGorelli !
There is no rush on our side, FYI :)

[MarcoGorelli]

Thanks a lot @MarcoGorelli !
There is no rush on our side, FYI :)

Fixed in version 0.9.0 🎉

It looks like doc/source/cookbook/fits_xray_images.ipynb now has a syntax error:

error: cannot format doc/source/cookbook/fits_xray_images.ipynb: Cannot parse: cell_12:2:125:                    [("fits", "flux"), ("fits", "projected_temperature"), ("gas", "pseudo_pressure"), ("gas", "pseudo_entropy"]), 
Oh no! 💥 💔 💥

Looks like the square and round brackets after "pseudo_entropy" are swapped got swapped in efb3411

[neutrinoceros]

Awesome, thank you so much for catching this !

[MarcoGorelli]

Happy to help, and thanks for your awesome library!

doc/source/cookbook/fits_xray_images.ipynb is still failing as it contains invalid syntax - I'd suggest either fixing that first, or (for now) using

-      args: [--nbqa-mutate]
+      args: [--nbqa-mutate, --nbqa-skip-bad-cells]

[cphyc]

@MarcoGorelli thanks! I've fixed the syntax error and the flake8 issues, it now seems to be working like a charm!

[cphyc]

pre-commit.ci autofix

[neutrinoceros]

@cphyc I believe the failures on Jenkins were fixed on the main branch, can you rebase this please ?

[cphyc]

pre-commit.ci autofix

[cphyc]

pre-commit.ci autofix

[neutrinoceros]

pre-commit.ci autofix

[neutrinoceros]

@cphyc looks like pre-commit.ci auto fixing is currently broken

[neutrinoceros]

Nevermind I get what's happening: most likely pre-commit.ci is timing out after 3min so it can't apply your changes.
If you don't want to assume authorship for a this sweeping change, I suggest you use a similar trick as I did with #2749 :

pre-commit run --all-files
git commit --author=convert-repo ...

Also, you may want to add the resulting commit to .git-blame-ignore-revs

[neutrinoceros]

Wow, black 21.8b0 now has a specialised pre-commit hook for notebooks,
psf/black#2357

@MarcoGorelli since you contributed this too, what's your recommendation ? is there any edge to keep using the nbqa hook or should we switch ?

[MarcoGorelli]

Hey @neutrinoceros, thanks for asking - I'd advise using black's hook, I'll deprecate the nbqa-black one

[MarcoGorelli]

black-jupyter does everything black does, but also checks notebooks - you probably just need that one

[neutrinoceros]

While you're at it @cphyc, you can remove the language_version line. It became useless in the latest release.

[matthewturk]

So where do we think we're at on this?

[cphyc]

So where do we think we're at on this?

This is good to go, but the tests failed randomly... I'll close/re-open to trigger them

[cphyc]

@yt-fido test this please