Jupytext plugin for the pre-commit package, compatible with black and flake8

[phaustin]

the pre-commit package (described in this blog) provides a framework for managing precommit hooks. It would be great if jupytext had an option that would allow it to be a pre-commit hook with that package -- essentially it would just need to suppress the stdout message and return 0, adding the translated file to the index. Specifically, this script is working for me now:

import sys
import subprocess
from pathlib import Path

cmd = f"jupytext --to py:percent {sys.argv[1]}"
notebook = Path(sys.argv[1])
pyfile = str(notebook.with_suffix(".py"))
status1, output1 = subprocess.getstatusoutput(cmd)
cmd = f"git add {pyfile}"
status2, output2 = subprocess.getstatusoutput(cmd)
with open("jupytext_logfile.txt", "w") as f:
    f.write(
        (f"{sys.argv}\n" f"{status1}\n" f"{output1}\n" f"{status2}\n" f"{output2}\n")
    )
sys.exit(0)

when I save it as pre_commit_jupytext.py and call it with a pre-commit config entry in
.pre-commit-config.yaml that looks like this:

repos
-   repo: local
    hooks:
    - id: jupytext
      name: jupytext
      entry: python a301/scripts/pre_commit_jupytext.py
      language: system

but I think smoothing this a little with a command line option would be a nice addition.

[mwouts]

Hello @phaustin , thanks for letting me know about the pre-commit package, which looks great!

We started exploring pre-commit hooks at #121 recently, and have implemented a --pre-commit mode in Jupytext command line, that is briefly documented here.

Is that pre-commit mode good for you?

Otherwise, you seem to be interested in a full python implementation of the conversion, right? Do you want to have a look here and try something like:

import sys
import jupytext

nbfile = Path(sys.argv[1])
pyfile = str(notebook.with_suffix(".py"))

nb = jupytext.readf(nbfile)
jupytext.writef(nb, pyfile, format='percent')

Please let me know which way is best for you!

[phaustin]

using the readf/writef pair is certainly cleaner that having to spawn a subprocess, thanks for that suggestion.

The issue I found with the jupytext --pre-commit flag is that the pre-commit package interprets the jupytext stdout message as a failure code, and won't proceed with the commit. Swallowing that output as part of my own commit script also allows me to commit the py:percent files to a python subfolder, which I think students will find easier to grep as they look for code tips from assignment notebooks. The pre-commit package also allows me to add flake8 and black to the hook.

Thanks for jupytext, I've been looking for something like this for quite a while.

[mwouts]

Oh, that's interesting! So if I summarize, you have prepared a flake8 + black pre-commit hook for notebooks? That's going to improve very significantly the quality of notebooks!! Do you plan to write a post about that ?

Now, returning to our discussion, do you think a quiet argument for Jupytext (or any other name you recommend) would be useful? Also, if you do plan to document how to use Jupytext with the pre-commit hook package, I would be happy to include either a link, or a short section about this in our readme (PR are welcome).

[phaustin]

1. on inserting black + flake8 into a notebook roundtrip -- yes, that's the idea, I think it would be useful to have both a stand-alone entry-point (say, "clean_notebook") and a pre-commit hook.

2. blog/writeup -- yes, I'm planning to document a lot of this over the next few months (i.e. turning notebooks into libraries, conda packaging, using nbgrader, etc.), as we put up instructional materials for teaching with jupyter notebooks at UBC. I'll also be making my remote sensing course: (https://clouds.eos.ubc.ca/~phil/courses/atsc301/coursebuild/notebook_index.html)
   and our numerical methods course:
   (https://clouds.eos.ubc.ca/~phil/numeric/lab_descriptions.html)
   more publically available as a template/source for examples

3. adding --pre-commit-quiet or something so that the following config would work with the pre-commit package:

% cat .pre-commit-config.yaml
repos:
-   repo: https://github.com/ambv/black
    rev: 18.9b0
    hooks:
    - id: black
      language_version: python3.7
      types: [python]
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v2.0.0
    hooks:
    - id: flake8
      types: [python]
-   repo: local
    hooks:
    - id: jupytext
      name: jupytext
      entry: jupytext --pre-commit-quiet
      language: system
      files: \.ipynb$

(not sure, but there might be another change needed as part of this to prevent a race condition as pre-commit and jupytext both try to add the python file to the index?)

This would allow a natural progression from standalone precommit hook to pre-commit package-registered hook, to a user-defined hook that does roundtrip cleaning.

[mwouts]

Well, that are very nice perspectives! Thanks also for sharing your course. I gave a quick look at it, and saw that you have a lot of material there.

For this issue, I will try to see

• how we could implement a 'black' and 'flake8' mode in jupytext for reformating/testing notebooks
• how we could offer a jupytext pre-commit hook for the pre-commit package, that
  • reformats with black, if desired
  • tests the notebook with flake8, if desired
  • creates or updates the python file corresponding to the notebook, possibly in a subfolder.

[mwouts]

I found a few implementations of black for notebooks at psf/black#298:

• nbblack: CLI that reformat all the cells of a notebook
• clean_ipynb: same as above - uses subprocess rather than python calls
• blackcellmagic: a magic cell for reformating a given cell.

[phaustin]

I found a few implementations of black for notebooks at ambv/black#298:

Definitely looks like there's a market for this -- happy to help with testing and spreading the word.

[mwouts]

Hello @phaustin , I see you are exporting the python representation of your notebooks to a python folder. That is a use case that I would like to support in the upcoming version 1.0. Can I ask your advice on how to encode this in the format specification?

I have been thinking of the following:

[prefix/][suffix.]ext:format_name

Therefore,

• py:percent would mean, as currently: export to a file with .py extension, in percent format
• _notebook.ipynb,_script.py would mean: assert that the notebook name ends with _notebook, remove _notebook from the file name, append _script and save to extension .py.
• ipynb,python_/py would pair the notebook to a .py file with prefix python_ in the same folder
• ipynb,python//py would pair the notebook to a file with an identical name, but in a sub folder python
• notebook//ipynb,python//py would assert that the notebook is in a folder ending with notebook, and save to a file with an identical name, but in a folder python.

Do you deem this as useful? Do you see any other alternative to the double slash for folders (single slash means prefix)?

[mwouts]

By the way: I plan to improve the interaction between jupytext and reformating tools with #154

[phaustin]

Hi Marc -- yes, we'd definitely like/use this. At the moment we're doing this by hand:
https://github.com/phaustin/eosc213_students

[mwouts]

Hello @phaustin . I am trying to figure out how jupytext could best work with the pre-commit package. I would like to be able to do each of the following:

1. convert the ipynb files in the index to py files
2. apply black, test flake8, update the ipynb back
3. optionally, remove ipynbs from the git index, as in Automatic conversion to .py upon git commit? #121

As I am just discovering the pre-commit package, may I ask you a few questions:
a. Do you have experience with compounding pre-commit hooks ? I mean, is it possible to apply multiple hooks in a specific order ? Do you see how one could do 1. and 2. above ? Or, do you recommend that we implement the black and flake8 calls in jupytext CLI itself ?
b. Are you aware of any hook that does 3. ? Should that be an option to jupytext CLI ?

Thanks !

[mwouts]

Hi @phaustin , I have prepared a release candidate for Jupytext 1.0. It has many new features, including the --quiet mode, the new --pipe argument, and path prefixes when pairing notebooks.

Could you please give a try to the new version, available with

pip install jupytext --pre --upgrade

and tell me if you can write a simple and satisfactory hook for the pre-commit package with the new release? Maybe we should also have a specific section on this in the documentation. Thanks!

[phaustin]

Yes, this is working well with our pre-commit workflow. Currently we

1. set .gitignore to ignore all ipynb files
2. set pre-commit to run black, flake8, reorder-python-imports and a custom hook on all py files
3. at the moment, the custom hook is using jupytext.(read,write) to strip instructor answers off of
   solution cells and insert a blank cell with a "your solution here" string
4. if we do want to commit a ipynb file, we turn off the pairing for that notebook with

   "formats": "ipynb",
   

[mwouts]

Hello @phaustin, again! I am trying to converge on release 1.0.0 (now with rc3), and was wondering how to resolve this issue.

You seem satisfied with the solution you describe above, and this is great ! Also, I've been experimenting on my side with the following pre-commit hooks:

jupytext --from ipynb --pre-commit --sync --pipe black 
jupytext --from ipynb --pre-commit --sync --quiet --pipe 'black - -q'

(the second being the quiet version), they work well and it's fun to black a notebook!

Now, do you think there could be a 'standard' jupytext pre-commit hook for the pre-commit hook package ? Possibly one of

jupytext --from ipynb --pre-commit --sync --quiet # (requires jupytext.formats metadata)
# or
jupytext --from ipynb --pre-commit --set-formats ipynb,py:percent --sync --quiet

Are they generic enough? Or, should we instead let the users write their own pre-commit hook using the many possible Jupytext's combinations ?

[phaustin]

Currently I think that by the time people adopt the pre-commit package they are beyond the simpler cases, and it's too hard to predict what their filter choices are going to be to bless one particular combination. For those users I believe the clearest approach is to let pre-commit handle the python files and jupytext the roundtrip conversion between python and notebooks.

[mwouts]

Sure! So we're on the same line. Thanks @phaustin for your feedback.

[jainr]

I am using following script for my pre-commit hook but it does not add the .py file (.pynb does does get added and it does the conversion to .py script) automatically to the commit. Am I missing something or this is not supported yet?

jupytext --from ipynb --to python//py:light --pre-commit

[mwouts]

Hello @jainr , thanks for reporting this. Your expectation is correct, so apparently we have a bug here. We will work on this at #200.

[jainr]

Thanks @mwouts

[KwatMDPhD]

How about this https://github.com/KwatME/clean_ipynb?

[mwouts]

Thanks @KwatME for sharing! I guess the difference between Jupytext + Black and clean_ipynb is that the first one treats the .py file globally, while I guess clean_ipynb treats each cell separately. In most cases the result should be the same - don't you think so?

[KwatMDPhD]

Agree. I think jupytext is wonderful because it can do so much and well liked by the community. I want to see how I can start using it :) Thanks @mwouts

[mwouts]

Thank you @KwatME for your kind words. Well you may be interested in the jupytext.read(s)/write(s) methods. They work similarly to nbformat.read(s)/write(s) (which you could use in place of json.load and json.dump to open/save notebook files).