Repository: mwouts/jupytext
Pull request #698 (2020-12-30 18:06:02)

Enable integration with pre-commit
This is a first attempt at an implementation of the suggestion in #691 to enable "proper" integration with the pre-commit framework. The changes summarized:
* A new `.pre-commit-hooks.yaml` file, which tells `pre-commit` how to install this package (pre-commit does this internally), which command to run, and what the default options are
* A new cli argument, `--add-untracked`, which adds any output files that weren't previously tracked by `git` to the index.  Two points to note here:
    * The reason we are tracking only new files is that `pre-commit` doesn't consider changes to them if they aren't in the index (pre-commit requires all files in the index be unchanged by all hooks before passing)
    * The command will return a non-zero status code in case any newly generated files are actually added. This is because by adding them "on the fly" during the hook, pre-commit doesn't pass them to any other configured hooks for further checks, so they could be wrongly accepted into the commit.

The net effect is a proper pre-commit hook, which will only allow a commit as long as the configured Jupytext command doesn't change any of the files in the index, or add any new ones. This is how pre-commit's hooks are expected to operate.

Questions:
* This is a bit of an adjustment to how Jupytext's main command is operating - is it too much? How easy would it be to make this separate/not expose the `--add-untracked` option so publicly?
* How to test this behaviour (will be investigating, this is a WIP)?

Skylion007: I was actually just about to make this PR myself, @JohnPatton. The untracked feature is nice, but isn't it similar to the `--precommit` arg already?
Skylion007: The proper way of doing this pre-commit hook is to allow pre-commit to pass in the args directly into jupytext by using pass_filenames: True. The only thing that may need to be improved is adding in a filtering that automatically adds relvant RST, py or other supported filetypes (currently that has to be specified if it's not ipynb).
mwouts: Hi @Skylion007 , thanks for joining the conversation! Well with @JohnPaton and yourself in the thread I am going to learn a LOT about pre-commit hooks, I am looking forward to it!

I don't know what you will think about that, but... I'd like to make the choices about this hook in a test-driven manner. I mean, I am thinking of a series of use cases that I'd like to see supported, like
1. all the notebooks in the repo are paired to `py:percent` files, and the `.ipynb` files are in `.gitignore`, so they don't reach the repository (the hook updates the `py` file from the `.ipynb` file that the user may be modifiying outside of Jupyter)
2. the notebooks are paired to `py:percent` files. The `.ipynb` files are committed with no outputs, but still the user can keep the outputs locally. And the hook ensures that modifications in whatever file are reported to the other one
3. the notebooks are paired to `py:percent` files, and synched with the `.ipynb` files. If a notebook was modified it is re-executed before the commit (I'm not so sure that this case makes sense - or do we want to reexecute the notebook only if the text version was changed?)

In addition to these use cases, I'd like to document and test (IMO the main purpose of writing the blog is to test that this is going to be usable :smile:) how to
- store the text versions and ipynb files in distinct folders (probably not related to the hook, this will directly depend on Jupytext)
- make sure that the `py` versions of the notebooks meets quality requirements (flake8) or formatting (black) - maybe this is not for our hook, but for a standard hook on `py` files
- and how to reformat the long lines in markdown cells into shorter ones, compliant with pep8

Which are your own expectations? How big is the overlap with mine?

Regarding the timing, I hope to have a draft for the blog by tomorrow or Friday, I'll share it with you both then, and then I hope to be able to iterate quite fast, within one or two weeks I mean.
Skylion007: In regards to three use cases:

1. This is fine as long as you don't modify the git staging. We could add an optional flag to throw errors if one of the edited files are not added to the index, instead of throwing it..  It doesn't matter if a file is in a gitignore if the file is added / currently staged for the commit.
1. Is actually very similar to what we do in habitat-sim and is accomplished by having two hooks (nbstripout being a seperate one to ensure all committed notebooks have no outputs). It won't edit the output of local files unless you try to commit them and those changes can be easily reverted / fixed with git pop/stash.
1. Is easy with default pre-commit settings, if you have --sync set, you can also have --execute. pre-commit will only execute on files that staged and changed in commit (unless manually run with --all-files).

By default, pre-commit hooks will only run on edited files for speed. Having it run on all files / the entire configuration is easily doable though.

* Yeah not related to the hook. Just the args that can be passed into the hook / the jupytext config.
* Easily done in both actually, with pipe + additional_requirements or by just editing the python files. I actually run them on both, but really only the editing the text files is necessary (requires two pre-commit runs in the later case).
* This seems unrelated and be can handled with pipe / additional_depenencies IMO. 

I'd say the overlap is quite large.
mwouts: John, Aaron, I've invited you to https://github.com/mwouts/pre_commit_hooks_for_jupyter_notebooks.
It is very much a work in progress, though.

The first thing that I'd like to chat about is this "test" (WIP as well):
https://github.com/mwouts/pre_commit_hooks_for_jupyter_notebooks/blob/ac60bcbcec5ff65c171f8bbb03cc13a795e88a7d/examples/py_percent_sync_with_ipynb_with_outputs/test_py_percent_sync_with_ipynb_with_outputs.py#L35-L64
```
def test_py_percent_sync_with_ipynb_with_outputs(tmpdir, capsys):
    # Create an empty repository and initialize it
    repo = Repo.init(tmpdir)

    # Add the .pre-commit-config.yaml that corresponds to this example
    create_pre_commit_config_yaml_file(tmpdir, __file__)
    repo.index.add([".pre-commit-config.yaml"])
    repo.index.commit("Add .pre-commit-config.yaml")

    # Install pre-commit
    with working_directory(tmpdir):
        pre_commit(["install"])

    # Add a .jupytext.toml file that pairs all notebooks to py:percent files
    tmpdir.join(".jupytext.toml").write_text(
        "default_jupytext_formats = ipynb,py:percent"
    )
    repo.index.add([".jupytext.toml"])
    repo.index.commit("Add .jupytext.toml")

    # Add an ipynb file
    # Try to commit => pre-commit should create the py file, and fail!
    # Try to commit => pre-commit should complain that the py file is not in the index
    # Add the py file to the index
    # And commit (OK)

    # Modify the py file
    # Try to commit => pre-commit should complain that the ipynb file is not in the index
    # Add the ipynb file
    # And commit (OK)
```

I like the idea to use such a test to document explicitly our expectations. But at this stage I am not sure that `pre_commit(["install"])` in the `tmpdir` works. Then, do my comments match what we said on the hook? Do you deem this as user friendly (I mean, failing often and requiring 'git add')?
Skylion007: @mwouts Should John and I edited the repo directly? Left some comments on the questions in the readme.
JohnPaton: I've given you write access to the PR @Skylion007 - I've got a pretty busy few days ahead so I'll be a bit less active than I'd like, sorry in advance
JohnPaton: Okay @mwouts @Skylion007, I've made some pretty big changes based on all the feedback (really feeling this collab guys, nice working with you!)

I removed `--add-untracked`, it's indeed wrong to modify the index. Instead, I've added two new options: 
* `--ignore-unmatched` so that `pre-commit` can pass the filenames and we can just ignore them if they don't match what we are trying to do
* and `--alert-untracked` which exits non-zero if the output files aren't tracked in the git index. That way we can always be sure that if the hook is making changes, either:
    * the files are tracked so `pre-commit` will fail the hook if they are modified, or 
    * the files are untracked, and `--alert-untracked` will fail the hook. 
   
  In each case the changes have to be added to the index before the hook can succeed

I have also removed default arguments beyond those two from the hook configuration, so that the user is forced to specify what they are trying to do. 

I still need to update the docs but wanted your feedback :) 
JohnPaton: codecov is complaining, but I'm not sure why
mwouts: Hi @JohnPaton , @Skylion007 , thank you so much for your work here, I also enjoy very much this collaboration!

I completely agree with the `--alert-untracked` argument, you've convinced me that it is a better idea than the previous `--add-untracked`.

Regarding the `--ignore-unmatched`, I am not so sure that it is necessary. I expect that `jupytext --sync filename.ext` does not do anything if filename.ext is not the name of a paired notebook (well it reads the file and then issues a warning, but no error). It only fails if `ext` is not a notebook extension (i.e. something else than `ipynb`, `md`, `py`, 'R', etc).

Also, while trying the hook I saw a potential issue with the new build system for Jupytext - @JohnPaton you might not have seen it before because it was introduced just last week. I'll report on that at #706 .
JohnPaton: I also hit that build issue now @mwouts because I rebased 😬 
JohnPaton: > I expect that jupytext --sync filename.ext does not do anything if filename.ext is not the name of a paired notebook

Hmmm, but if they are not using `--sync`, but `--from --to`, then it does throw errors
mwouts: > Hmmm, but if they are not using --sync, but --from --to, then it does throw errors

Oh yes that's right... By the way I wanted to ask why you don't add `--sync` to the defaut command, but I think your comment answers my question, you want to run `--to` in some cases.

Regarding the build issue, I'll try to address it tonight. Meanwhile a quick fix would be to comment out the part of `setup.py` that depends on `jupyter-packaging` - you won't need that part of the package (the extension for Jupyter Lab) in the pre-commit hook anyway.
codecov[bot]: # [Codecov](https://codecov.io/gh/mwouts/jupytext/pull/698?src=pr&el=h1) Report
> Merging [#698](https://codecov.io/gh/mwouts/jupytext/pull/698?src=pr&el=desc) (f10f099) into [master](https://codecov.io/gh/mwouts/jupytext/commit/28807b383836f2ddab52d9448655e4846aa7c58d?el=desc) (28807b3) will **decrease** coverage by `1.36%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/mwouts/jupytext/pull/698/graphs/tree.svg?width=650&height=150&src=pr&token=a1pNjsiILi)](https://codecov.io/gh/mwouts/jupytext/pull/698?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #698      +/-   ##
==========================================
- Coverage   99.12%   97.76%   -1.37%     
==========================================
  Files          95       95              
  Lines        9356     9471     +115     
==========================================
- Hits         9274     9259      -15     
- Misses         82      212     +130     
```


| [Impacted Files](https://codecov.io/gh/mwouts/jupytext/pull/698?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [jupytext/cli.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-anVweXRleHQvY2xpLnB5) | `96.62% <100.00%> (+0.17%)` | :arrow_up: |
| [tests/test\_pre\_commit.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9wcmVfY29tbWl0LnB5) | `92.90% <100.00%> (-7.10%)` | :arrow_down: |
| [tests/utils.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-dGVzdHMvdXRpbHMucHk=) | `100.00% <100.00%> (ø)` | |
| [jupytext/pandoc.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-anVweXRleHQvcGFuZG9jLnB5) | `51.72% <0.00%> (-44.83%)` | :arrow_down: |
| [tests/test\_read\_simple\_pandoc.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9yZWFkX3NpbXBsZV9wYW5kb2MucHk=) | `57.89% <0.00%> (-42.11%)` | :arrow_down: |
| [tests/test\_cli.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9jbGkucHk=) | `93.08% <0.00%> (-6.92%)` | :arrow_down: |
| [tests/test\_formats.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9mb3JtYXRzLnB5) | `96.74% <0.00%> (-3.26%)` | :arrow_down: |
| [jupytext/jupytext.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-anVweXRleHQvanVweXRleHQucHk=) | `96.46% <0.00%> (-2.22%)` | :arrow_down: |
| [tests/test\_contentsmanager.py](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9jb250ZW50c21hbmFnZXIucHk=) | `98.99% <0.00%> (-1.01%)` | :arrow_down: |
| ... and [2 more](https://codecov.io/gh/mwouts/jupytext/pull/698/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mwouts/jupytext/pull/698?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/mwouts/jupytext/pull/698?src=pr&el=footer). Last update [28807b3...354a9a5](https://codecov.io/gh/mwouts/jupytext/pull/698?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

mwouts: Hi @JohnPaton , @Skylion007 , well the PR is going well, isn't it? I think we've done most of the review, and must say that I like very much the contribution, and also the tests that you have added here.

Maybe there are two more small points that I'd like to see with you:
- I think that the error message when one file created by Jupytext is untracked is super important. Can we make it very clear that the user should simply run `git add` on that file?
- Also, the tests are for `--from ipynb --to py`, and that is a very nice use case, but I think that `--sync` is going to be more idiomatic. Do you have reasons to prefer the first form? Also, would you mind adding one test for that mode? Note that you will need to first turn the `py` or `ipynb` file into a paired notebook, which you can do with either a `jupytext.toml` file with this content: `"default_jupytext_formats = ipynb,py:percent"`, or with a call to `jupytext --set-formats ipynb,py:percent` on that file.

Also this is the first time I see a PR with a "draft" status, who is supposed to change it to "ready", is that you, @JohnPaton? On my side I'll continue working on the blog post but maybe we don't need to further postpone this PR, with the tests I think that we can have a good confidence that this is going to work well!

 Another question, probably I should plan to tag/release a new version when we integrate this?
JohnPaton: > Can we make it very clear that the user should simply run git add on that file?

Agreed, done, let me know if this is good enough!

> Do you have reasons to prefer the first form?

Because my goal in using Jupytext is to get rid of Notebooks from our VCS entirely, even if users start their work by creating them. But I can switch one of the tests, sure! Unfortunately I can't verify that it works because I can no longer install Jupytext (blocked by #706), so my ability to contribute much more is a bit limited. 

> who is supposed to change it to "ready"

I'll do that now!

> Another question, probably I should plan to tag/release a new version when we integrate this?

I think so, we are introducing some new CLI args so probably a good idea
mwouts: Hi @JohnPaton , I did some testing, and I think we need a few more updates to make sure that the `--sync` mode works well.

Also, I've started adding a few more commits on top of yours at https://github.com/mwouts/jupytext/commits/JohnPaton-pre-commit-hooks-2, but now the commit does not fail, I guess that I have not setup `types` correctly - do I need to add them to the `.pre-commit-config.yaml` as well?

> Because my goal in using Jupytext is to get rid of Notebooks from our VCS entirely
Ha ha! Well that is a point that I'd like to chat a bit more about... For instance, do you plan to remove the `ipynb` file from the index? In the documentation of the other implementation of pre-commit hook, we do have a line that says that one can run `git reset HEAD **/*.ipynb` at the end of the hook. Do you plan to do something similar? By the way, let me recall that a simple approach for that is to use _paired notebooks_ (the synchronisation is done by Jupyter itself) and then just add `*.ipynb` to `.gitignore`.
JohnPaton: Hi @mwouts, I've rebased again with your changes and now it installs (yay!), but I can't run the tests still. Any ideas?

```console
$ pytest tests/test_pre_commit.py
ImportError while loading conftest '/Users/cw0504/personal/jupytext/tests/conftest.py'.
tests/conftest.py:15: in <module>
    jupytext.config.JUPYTEXT_CEILING_DIRECTORIES = ["/tmp/"]
E   AttributeError: module 'jupytext' has no attribute 'config'
exit 4
```
mwouts: Hi @JohnPaton , I'm so sorry for this extra difficulty.

Locally I can't reproduce the `AttributeError: module 'jupytext' has no attribute 'config'` error.
Thus I would advice to recreate your dev environment from scratch, following [these instructions](https://github.com/mwouts/jupytext/blob/master/CONTRIBUTING.md#jupytext-as-a-python-package), i.e.
```
conda env create --file environment.yml  # or conda env update --file ...
conda activate jupytext-dev
python -m ipykernel install --name jupytext-dev --user
pip install -e .
```

Also, you've done a lot of work already - if you prefer we can ship the version that works with `--to` and do a lighter follow-up on `--sync`.
JohnPaton: I'd rather just get it right, especially if `--sync` is the most common use case 😬  Indeed looks like my env was borked, recreating did the trick.

mwouts: Hi @JohnPaton , well `--sync` first finds out which are the files paired with the current notebook. This information may come from the notebook metadata (`jupytext.formats`) or from the `jupytext.toml` configuration file.

Then, it determines which file is the most recent in this set, and then it updates the inputs cells of the other files with these most recent inputs.

In other words, if the `.ipynb` file was last modified, then it will update the `.py` file with the inputs cells from the `.ipynb` file. And if the `.py` file was last modified, the input cells of the `.ipynb` files are updated, using the content from the `.py` file (and outputs in the `.ipynb` are preserved, as much as possible).

The `--sync` mode is an attempt to replicate the Jupytext plugin (contents manager) for Jupyter at the command line. For an introduction to that one, you can have a look at this 12 minute presentation: [![](https://img.shields.io/badge/YouTube-JupyterCon%202020-red.svg)](https://www.youtube.com/watch?v=SDYdeVfMh48)
JohnPaton: Almost there I think. The one last problem is that the hook seems to be updating both files in `--sync` mode (I was expecting only the older file to be updated). Because both files are being updated, the hook doesn't know which set of updates to take, so it fails... thoughts? 

```
jupytext.................................................................Failed
- hook id: jupytext
- files were modified by this hook

[jupytext] Ignoring unmatched input path .pre-commit-config.yaml
[jupytext] Reading test.ipynb in format ipynb
[jupytext] Updating 'test.py'
[jupytext] Reading test.py in format py
[jupytext] Loading 'test.ipynb'
[jupytext] Updating 'test.ipynb'
[jupytext] Updating 'test.py'

[WARNING] Stashed changes conflicted with hook auto-fixes... Rolling back fixes...
```
mwouts: Thank you @JohnPaton , you've done a hard work here!

Well you're right, when the `.py` file is more recent than the `.ipynb` file, `--sync` updates the `.ipynb` file and then updates (the timestamp) of the `.py` file.

Now I'll try to explain the reason for that... it is rooted in the other usage of Jupytext - when Jupytext is used within Jupyter. In that context, the user may edit either the `.py` file or the `.ipynb` file. We always take the `.py` file as the source for inputs, and we **throw an error** (we ask the user to delete one of the two files) if the `.py` file is older than the `.ipynb` file. This works (and is compatible with `--sync`, that was developed later on) as Jupytext always writes the `.ipynb` file before the `.py` version.

Maybe my preferred way to solve this would be, thus, to find a way to update the timestamp of the `.py` file (currently done at https://github.com/mwouts/jupytext/blob/master/jupytext/cli.py#L691-L693 by actually writing the file) that does not attract the attention of `pre-commit`. Do you think this is an option?

Another option is to assume that users of the pre-commit hook won't be using Jupytext's contents manager, and to skip the update of the unmodified file.

Or maybe the problem will exist anyway (`.ipynb` more recent than `.py` especially on large files) when we pull a commit from a remote? If so, maybe we could skip the update of the unmodified file, and use a post-commit hook to finally update the (timestamp) of the `.py` file to make it more recent than the `.ipynb`?

@Skylion007 , did that an issue appear in the local version of the hook? Do you have any advice on how to address this? Thanks
Skylion007: Yeah, to be honest the way I solve this is by deleting one of the files and
then rerunning it. It's not elegant, but it works. Yeah this is one of the
biggest issues with using it as a pre-commit hook currently. Updating the
timestamps of both files might be the way to go here. We can safely do that
now that we require_serial: True in our JuPyText hook.

We don't need to actually write the file btw, we can set the date modified
by using os.utime https://docs.python.org/3/library/os.html#os.utime

I prefer this solution as it still satisfies the JuPyText contents manager
and frankly both files should be updated when one is modified (even if the
update is a NOOP). This behavior could be changed by a flag (we may want to
group all these behaviors under a master flag pre-commit mode).

On Mon, Jan 18, 2021 at 6:16 PM Marc Wouts <notifications@github.com> wrote:

> Thank you @JohnPaton <https://github.com/JohnPaton> , you've done a hard
> work here!
>
> Well you're right, when the .py file is more recent than the .ipynb file,
> --sync updates the .ipynb file and then updates (the timestamp) of the .py
> file.
>
> Now I'll try to explain the reason for that... it is rooted in the other
> usage of Jupytext - when Jupytext is used within Jupyter. In that context,
> the user may edit either the .py file or the .ipynb file. We always take
> the .py file as the source for inputs, and we *throw an error* (we ask
> the user to delete one of the two files) if the .py file is older than
> the .ipynb file. This works (and is compatible with --sync, that was
> developed later on) as Jupytext always writes the .ipynb file before the
> .py version.
>
> Maybe my preferred way to solve this would be, thus, to find a way to
> update the timestamp of the .py file (currently done at
> https://github.com/mwouts/jupytext/blob/master/jupytext/cli.py#L691-L693
> by actually writing the file) that does not attract the attention of
> pre-commit. Do you think this is an option?
>
> Another option is to assume that users of the pre-commit hook won't be
> using Jupytext's contents manager, and to skip the update of the unmodified
> file.
>
> Or maybe the problem will exist anyway (.ipynb more recent than .py
> especially on large files) when we pull a commit from a remote? If so,
> maybe we could skip the update of the unmodified file, and use a
> post-commit hook to finally update the (timestamp) of the .py file to
> make it more recent than the .ipynb?
>
> @Skylion007 <https://github.com/Skylion007> , did that an issue appear in
> the local version of the hook? Do you have any advice on how to address
> this? Thanks
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/mwouts/jupytext/pull/698#issuecomment-762509399>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAPVMX2PHQO3KGFFZ765P4TS2S6LFANCNFSM4VOSIZLQ>
> .
>

mwouts: > We don't need to actually write the file btw, we can set the date modified by using os.utime

Yes! I'll try to do that tonight (don't rewrite the `.py` file but just update the last modification time). I think it should be the default mode in `--sync` - no need for something specific to the pre-commit mode here, hopefully. But... will that work with pre-commit? How does pre-commit detect that the file has changed?
JohnPaton: If it was only timestamps, it would notify that the timestamp is being updated, right? I don't see any
```python
"[jupytext] Sync timestamp of '{}'".format(nb_file)
```
line in my output
Skylion007: I haven't checked, but I believe pre-commit uses git to see which files don't match the index. It looks like git doesn't store any metadata about the file like ACLs or modified time so that might work: https://stackoverflow.com/a/2179825
mwouts: Hi @JohnPaton , maybe this commit https://github.com/mwouts/jupytext/commit/22d6961572f6367945189d986f813ccd701f6084 can help? It generalizes the timestamp update (already there for the `--to` mode) to the `--sync` mode.
JohnPaton: I'm afraid that wasn't enough either, it seems like both files are still being modified
```
jupytext.................................................................Failed
- hook id: jupytext
- files were modified by this hook

[jupytext] Ignoring unmatched input path .pre-commit-config.yaml
[jupytext] Reading test.ipynb in format ipynb
[jupytext] Updating 'test.py'
[jupytext] Reading test.py in format py
[jupytext] Loading 'test.ipynb'
[jupytext] Updating 'test.ipynb'
[jupytext] Sync timestamp of 'test.py'

[WARNING] Stashed changes conflicted with hook auto-fixes... Rolling back fixes...
```

Not sure if this matters: `pre-commit` passes the files individually, iiuc, so it will call `jupytext --sync test.ipynb` and then `jupytext --sync test.py`
mwouts: Hi @JohnPaton , yes you're right the issue is with the fact that `jupytext --sync` is called twice. On the second call the `.py` file is more recent so the `.ipynb` file is updated.

Here are a few workarounds that I am thinking of...
1. run the hook only on `.ipynb` files
2. call jupytext just once on the set of all files (and change `jupytext/cli.py` to run `jupytext` exactly once per paired notebook)
3. update `jupytext/cli.py` again, and skip the calls to 'write' if we see that the updated version matches the current file

I think that 1. is clearly the simple approach. And FYI I might also consider 3 because I think it will be required in case of triple-paired notebooks (`.ipynb` + `.md` + `.py`) (but be reassured, we will keep that for later on)
JohnPaton: Alright, let's go with option 1 for now then! Then we can get this bad boy merged 🚀 
mwouts: Hi @JohnPaton , @Skylion007 , we're getting ready!

I am done with the review and I'd like to discuss a few details:
1. The example with `--to` in the doc needs to be updated to match the one actually tested (use e.g. https://github.com/mwouts/jupytext/commit/07b323b92da78d24b7f6c0887adc8f9d30607f8f)
2. Maybe we should temporarily remove the `--pipe` example from the documentation - at least I can't get it to work in the tests for now. I think option 3 from my previous comment could help here, but I'd prefer to re-add the example later on when it is ready.
3. How do you want to see this merged? Should I squash or rebase? If we squash, are you both OK for giving the full authorship to John (and I'll cite you both in the changelog)? Sorry for asking, it is the first time I see a PR with so many commits and contributors :smile: 
4. Github says that there is a conflict with the base branch, is it easy for you to solve? (Well if we decide to squash, don't bother with that, as I should be able to do it locally)
Skylion007: @mwouts Squashing commits will preserve authorship if done correctly (via the co-author tag in the commit description).

>> update jupytext/cli.py again, and skip the calls to 'write' if we see that the updated version matches the current file
I'd much prefer 3 in the long term. It's sane behavior.

Patch:
diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml
new file mode 100644
index 00000000..6faaadc4
--- /dev/null
+++ b/.pre-commit-hooks.yaml
@@ -0,0 +1,8 @@
+- id: jupytext
+  name: jupytext
+  description: Runs jupytext on all notebooks and paired files.
+  language: python
+  entry: jupytext --ignore-unmatched --alert-untracked
+  pass_filenames: true
+  require_serial: true
+  types: [jupyter]
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index f61df207..3be52595 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -5,13 +5,14 @@ Jupytext ChangeLog
 ---------------
 
 **Changed**
-- Jupytext does not work properly with the new cell ids of the version 4.5 of `nbformat>=5.1.0` yet, so we added the requirement `nbformat<=5.0.8` (#715)
+- Jupytext does not work properly with the new cell ids of the version 4.5 of `nbformat>=5.1.0` yet, so we added the requirement `nbformat<=5.0.8` ([#715](https://github.com/mwouts/jupytext/issues/715))
+- `jupytext --sync` only updates the timestamp of the text file (not the file itself) when that file is the most recent ([#698](https://github.com/mwouts/jupytext/issues/698))
 
 **Fixed**
-- Indented magic commands are supported (#694)
+- Indented magic commands are supported ([#694](https://github.com/mwouts/jupytext/issues/694))
 
 **Added**
-- Added a test that ensures that `py:percent` scripts end with exactly one blank line (#682)
+- Added a test that ensures that `py:percent` scripts end with exactly one blank line ([#682](https://github.com/mwouts/jupytext/issues/682))
 
 
 1.9.1 (2021-01-06)
diff --git a/docs/using-pre-commit.md b/docs/using-pre-commit.md
index 402d67cc..29c1907d 100644
--- a/docs/using-pre-commit.md
+++ b/docs/using-pre-commit.md
@@ -27,35 +27,43 @@ Note that these hooks do not update the `.ipynb` notebook when you pull. Make su
 
 ## Using Jupytext with the pre-commit package manager
 
-Using Jupytext with the [pre-commit package manager](https://pre-commit.com/) is another option. You could add the following to your `.pre-commit-config.yaml` file:
-```
+Using Jupytext with the [pre-commit package manager](https://pre-commit.com/) is another option. You could add the following to your `.pre-commit-config.yaml` file to sync all staged notebooks:
+
+```yaml
 repos:
--   repo: local
+-   repo: https://github.com/mwouts/jupytext
+    rev: #CURRENT_TAG/COMMIT_HASH
     hooks:
     - id: jupytext
-      name: jupytext
-      entry: jupytext --to md
-      files: .ipynb
-      language: python
+      args: [--sync]
 ```
 
-Here is another `.pre-commit-config.yaml` example that uses the --pre-commit mode of Jupytext to convert all `.ipynb` notebooks to `py:light` representation and unstage the `.ipynb` files before committing.
+You can provide almost all command line arguments to Jupytext in pre-commit, for example to produce several kinds of output files:
+
+```yaml
+repos:
+-   repo: https://github.com/mwouts/jupytext
+    rev: #CURRENT_TAG/COMMIT_HASH
+    hooks:
+    - id: jupytext
+      args: [--from, ipynb, --to, py:light, --to, markdown]
 ```
+
+If you are combining Jupytext with other pre-commit hooks, you must ensure that all hooks will pass on any files you generate. For example, if you have a hook for using `black` to format all your python code, then you should use Jupytext's `--pipe` option to also format newly generated Python scripts before writing them:
+
+```yaml
 repos:
-  -
-    repo: local
+-   repo: https://github.com/mwouts/jupytext
+    rev: #CURRENT_TAG/COMMIT_HASH
     hooks:
-      -
-        id: jupytext
-        name: jupytext
-        entry: jupytext --from ipynb --to py:light --pre-commit
-        pass_filenames: false
-        language: python
-      -
-        id: unstage-ipynb
-        name: unstage-ipynb
-        entry: git reset HEAD **/*.ipynb
-        pass_filenames: false
-        language: system
+    - id: jupytext
+      args: [--sync, --pipe, black]
+      additional_dependencies:
+        - black==19.10b0 # Matches hook
 
+-   repo: https://github.com/psf/black
+    rev: 19.10b0
+    hooks:
+    - id: black
+      language_version: python3
 ```
diff --git a/jupytext/cli.py b/jupytext/cli.py
index 4c2db448..72424dd6 100644
--- a/jupytext/cli.py
+++ b/jupytext/cli.py
@@ -87,6 +87,18 @@ def parse_jupytext_args(args=None):
         "on the notebooks found in the git index, which have an "
         "extension that matches the (optional) --from argument.",
     )
+    parser.add_argument(
+        "--ignore-unmatched",
+        action="store_true",
+        help="Ignore passed filepaths that aren't in the source format "
+        "you are trying to convert from.",
+    )
+    parser.add_argument(
+        "--alert-untracked",
+        action="store_true",
+        help="Exit with a non-zero status if the output files are not "
+        "tracked in the git index",
+    )
     parser.add_argument(
         "--from",
         dest="input_format",
@@ -417,15 +429,29 @@ def jupytext_single_file(nb_file, args, log):
             msg += " Maybe you mean 'jupytext --sync {}' ?".format(args.set_formats)
         raise ValueError(msg)
 
-    nb_dest = args.output or (
-        None
-        if not args.output_format
-        else (
-            "-"
-            if nb_file == "-"
-            else full_path(base_path(nb_file, args.input_format), args.output_format)
-        )
-    )
+    nb_dest = None
+    if args.output:
+        nb_dest = args.output
+    elif args.output_format and nb_file == "-":
+        nb_dest = "-"
+    else:
+        try:
+            bp = base_path(nb_file, args.input_format)
+        except InconsistentPath:
+            if args.ignore_unmatched:
+                log(
+                    "[jupytext] Ignoring unmatched input path {}{}".format(
+                        nb_file,
+                        " for format {}".format(args.input_format)
+                        if args.input_format
+                        else "",
+                    )
+                )
+                return 0
+            else:
+                raise
+        if args.output_format:
+            nb_dest = full_path(bp, args.output_format)
 
     config = load_jupytext_config(os.path.abspath(nb_file))
 
@@ -688,18 +714,37 @@ def jupytext_single_file(nb_file, args, log):
         if modified:
             inputs_nb_file = outputs_nb_file = None
 
+        untracked_paths = []
+
         def write_function(path, fmt):
-            # Do not write the ipynb file if it was not modified
-            # But, always write text representations to make sure they are the most recent
-            if path == inputs_nb_file and path == outputs_nb_file:
+            if path == inputs_nb_file:
+                # We update the timestamp of the text file to make sure it remains more recent than the ipynb
+                if path != outputs_nb_file:
+                    log("[jupytext] Sync timestamp of '{}'".format(nb_file))
+                    os.utime(nb_file, None)
+
+                # We don't write the ipynb file if it was not modified
                 return
+
             log("[jupytext] Updating '{}'".format(path))
             write(notebook, path, fmt=fmt)
             if args.pre_commit:
                 system("git", "add", path)
+            if args.alert_untracked and is_untracked(path):
+                nonlocal untracked_paths
+                untracked_paths.append(path)
 
         formats = prepare_notebook_for_save(notebook, config, nb_file)
         write_pair(nb_file, formats, write_function)
+        if untracked_paths:
+            log(
+                "[jupytext] Output file {nb_dest} is not tracked in the git index, "
+                "add it to the index using 'git add' to fix this.".format(
+                    nb_dest=", ".join(untracked_paths)
+                )
+            )
+            return 1
+
     elif (
         os.path.isfile(nb_file)
         and nb_dest.endswith(".ipynb")
@@ -711,7 +756,14 @@ def jupytext_single_file(nb_file, args, log):
         log("[jupytext] Sync timestamp of '{}'".format(nb_file))
         os.utime(nb_file, None)
 
-    return 0
+    if args.alert_untracked and is_untracked(nb_dest):
+        log(
+            "[jupytext] Output file {nb_dest} is not tracked in the git index, "
+            "add it to the index using 'git add' to fix this.".format(nb_dest=nb_dest)
+        )
+        return 1
+    else:
+        return 0
 
 
 def notebooks_in_git_index(fmt):
@@ -731,6 +783,14 @@ def notebooks_in_git_index(fmt):
     return files
 
 
+def is_untracked(filepath):
+    """Check whether a file is untracked by the git index"""
+    if not filepath:
+        return False
+    output = system("git", "ls-files", filepath).strip()
+    return output == ""
+
+
 def print_paired_paths(nb_file, fmt):
     """Display the paired paths for this notebook"""
     notebook = read(nb_file, fmt=fmt)
diff --git a/requirements-dev.txt b/requirements-dev.txt
index b5bf1992..7b6dd077 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -10,6 +10,7 @@ setuptools
 toml
 jupyterlab==3.0.0
 jupyter-packaging
+pre-commit
 
 # Python 2
 pathlib


Test patch:
diff --git a/tests/test_metadata_filter.py b/tests/test_metadata_filter.py
index 377174e8..6aababec 100644
--- a/tests/test_metadata_filter.py
+++ b/tests/test_metadata_filter.py
@@ -156,12 +156,11 @@ def test_default_config_has_priority_over_current_metadata(
 
     cfg_file = tmpdir.join("jupytext.toml")
     cfg_file.write(
-        """default_jupytext_formats = "ipynb,py:percent"
-default_cell_metadata_filter = "-some_metadata_key"
+        """default_cell_metadata_filter = "-some_metadata_key"
 """
     )
 
-    jupytext_cli([str(py_file), "--sync"])
+    jupytext_cli([str(py_file), "--to", "py"])
     assert (
         py_file.read()
         == """# %%
diff --git a/tests/test_pre_commit.py b/tests/test_pre_commit.py
index a73cc6a3..c607eadd 100644
--- a/tests/test_pre_commit.py
+++ b/tests/test_pre_commit.py
@@ -2,18 +2,25 @@
 
 import os
 import stat
+from pathlib import Path
+from textwrap import dedent
 
 import unittest.mock as mock
 import pytest
 from jupytext.compare import compare
 from nbformat.v4.nbbase import new_notebook, new_markdown_cell, new_code_cell
 from jupytext import read, write
-from jupytext.cli import jupytext, system
+from jupytext.cli import jupytext, system, is_untracked
 from jupytext.compare import compare_notebooks
 from .utils import list_notebooks
-from .utils import requires_black, requires_flake8, requires_pandoc
+from .utils import requires_black, requires_flake8, requires_pandoc, requires_pre_commit
 from .utils import requires_jupytext_installed
 
+try:
+    from pre_commit.main import main as pre_commit
+except (ImportError, ModuleNotFoundError):
+    pre_commit = None
+
 
 def git_in_tmpdir(tmpdir):
     """Return a function that will execute git instruction in the desired directory"""
@@ -31,6 +38,15 @@ def git_in_tmpdir(tmpdir):
     return git
 
 
+def system_in_tmpdir(tmpdir):
+    """Return a function that will execute system commands in the desired directory"""
+
+    def system_(*args):
+        return system(*args, cwd=str(tmpdir))
+
+    return system_
+
+
 @requires_jupytext_installed
 def test_pre_commit_hook(tmpdir):
     tmp_ipynb = str(tmpdir.join("nb with spaces.ipynb"))
@@ -252,13 +268,10 @@ def test_manual_call_of_pre_commit_hook(tmpdir):
     nb = new_notebook(cells=[])
     os.chdir(str(tmpdir))
 
-    def system_in_tmpdir(*args):
-        return system(*args, cwd=str(tmpdir))
-
     git = git_in_tmpdir(tmpdir)
 
     def hook():
-        with mock.patch("jupytext.cli.system", system_in_tmpdir):
+        with mock.patch("jupytext.cli.system", system_in_tmpdir(tmpdir)):
             jupytext(["--to", "py", "--pre-commit"])
 
     write(nb, tmp_ipynb)
@@ -364,3 +377,197 @@ def test_wrap_markdown_cell(tmpdir):
     text = nb.cells[0].source
     assert len(text.splitlines()) >= 2
     assert text != long_text
+
+
+def test_is_untracked(tmpdir):
+    git = git_in_tmpdir(tmpdir)
+
+    # make a test file
+    file = tmpdir.join("test.txt")
+    file.write("test file\n")
+    file = str(file)
+
+    with tmpdir.as_cwd():
+        # untracked
+        assert is_untracked(file)
+
+        # added, not committed
+        git("add", file)
+        assert not is_untracked(file)
+
+        # committed
+        git("commit", "-m", "'test'")
+        assert not is_untracked(file)
+
+
+def test_ignore_unmatched_ignores(tmpdir):
+    # Unmatched file
+    file = tmpdir.join("test.txt")
+    file.write("Hello\n")
+
+    # Run jupytext
+    status = jupytext(
+        ["--from", "ipynb", "--to", "py:light", "--ignore-unmatched", str(file)]
+    )
+
+    assert status == 0
+    assert not tmpdir.join("test.py").exists()
+
+
+def test_alert_untracked_alerts(tmpdir):
+    git_in_tmpdir(tmpdir)
+
+    file = tmpdir.join("test.py")
+    file.write("print('hello')\n")
+
+    # Run jupytext
+    with tmpdir.as_cwd():
+        status = jupytext(
+            ["--from", ".py", "--to", "ipynb", "--alert-untracked", str(file)]
+        )
+
+    assert status != 0
+    assert tmpdir.join("test.ipynb").exists()
+
+
+def test_alert_untracked_alerts_when_using_sync(tmpdir):
+    git_in_tmpdir(tmpdir)
+
+    file = tmpdir.join("test.py")
+    file.write("print('hello')\n")
+
+    tmpdir.join(".jupytext.toml").write('default_jupytext_formats = "ipynb,py"')
+
+    # Run jupytext
+    with tmpdir.as_cwd():
+        status = jupytext(["--sync", "--alert-untracked", str(file)])
+
+    assert status != 0
+    assert tmpdir.join("test.ipynb").exists()
+
+
+def test_alert_untracked_not_alerts_for_tracked(tmpdir):
+    git = git_in_tmpdir(tmpdir)
+
+    # write test notebook
+    nb = new_notebook(cells=[new_markdown_cell("A short notebook")])
+    nb_file = str(tmpdir.join("test.ipynb"))
+    write(nb, nb_file)
+
+    # write existing output
+    tmpdir.join("test.py").write("# Hello")
+
+    # track output file
+    git("add", str("test.py"))
+
+    # Run jupytext
+    with tmpdir.as_cwd():
+        status = jupytext(
+            ["--from", "ipynb", "--to", "py:light", "--alert-untracked", str(nb_file)]
+        )
+
+    assert status == 0
+
+
+@requires_pre_commit
+def test_pre_commit_hook_for_new_file(tmpdir):
+    # get the path and revision of this repo, to use with pre-commit
+    repo_root = str(Path(__file__).parent.parent.resolve())
+    repo_rev = system("git", "rev-parse", "HEAD", cwd=repo_root).strip()
+
+    # set up the tmpdir repo with pre-commit
+    git = git_in_tmpdir(tmpdir)
+
+    pre_commit_config_yaml = dedent(
+        f"""
+        repos:
+        - repo: {repo_root}
+          rev: {repo_rev}
+          hooks:
+          - id: jupytext
+            args: [--sync]
+        """
+    )
+    tmpdir.join(".pre-commit-config.yaml").write(pre_commit_config_yaml)
+    git("add", ".pre-commit-config.yaml")
+    with tmpdir.as_cwd():
+        pre_commit(["install", "--install-hooks", "-f"])
+
+    # write test notebook and sync it to py:percent
+    nb = new_notebook(cells=[new_markdown_cell("A short notebook")])
+    nb_file = str(tmpdir.join("test.ipynb"))
+    py_file = str(tmpdir.join("test.py"))
+    write(nb, nb_file)
+
+    with tmpdir.as_cwd():
+        jupytext(["--set-formats", "ipynb,py:percent", nb_file])
+
+    # try to commit it, should fail since the hook runs and makes changes
+    git("add", nb_file)
+    with pytest.raises(SystemExit):
+        git("commit", "-m", "failing")
+
+    # try again, it will still fail because the output hasn't been added
+    with pytest.raises(SystemExit):
+        git("commit", "-m", "still failing")
+
+    # add the new file, now the commit will succeed
+    git("add", py_file)
+    git("commit", "-m", "passing")
+    assert "test.ipynb" in git("ls-files")
+    assert "test.py" in git("ls-files")
+
+
+@requires_pre_commit
+def test_pre_commit_hook_for_existing_changed_file(tmpdir):
+    # get the path and revision of this repo, to use with pre-commit
+    repo_root = str(Path(__file__).parent.parent.resolve())
+    repo_rev = system("git", "rev-parse", "HEAD", cwd=repo_root).strip()
+
+    git = git_in_tmpdir(tmpdir)
+
+    # set up the tmpdir repo with pre-commit
+    pre_commit_config_yaml = dedent(
+        f"""
+        repos:
+        - repo: {repo_root}
+          rev: {repo_rev}
+          hooks:
+          - id: jupytext
+            args: [--from, ipynb, --to, "py:light"]
+        """
+    )
+    tmpdir.join(".pre-commit-config.yaml").write(pre_commit_config_yaml)
+    git("add", ".pre-commit-config.yaml")
+    with tmpdir.as_cwd():
+        pre_commit(["install", "--install-hooks"])
+
+    # write test notebook and output file
+    nb = new_notebook(cells=[new_markdown_cell("A short notebook")])
+    nb_file = str(tmpdir.join("test.ipynb"))
+    write(nb, nb_file)
+    py_file = str(tmpdir.join("test.py"))
+    jupytext(["--from", "ipynb", "--to", "py:light", str(nb_file)])
+
+    git("add", ".")
+    git("commit", "-m", "test")
+
+    # make a change to the notebook
+    nb = new_notebook(cells=[new_markdown_cell("Some other text")])
+    write(nb, nb_file)
+
+    git("add", nb_file)
+    # now a commit will fail, and keep failing until we add the new
+    # changes made to the existing output to the index ourselves
+    with pytest.raises(SystemExit):
+        git("commit", "-m", "fails")
+
+    with pytest.raises(SystemExit):
+        git("commit", "-m", "fails again")
+
+    # once we add the changes, it will pass
+    git("add", py_file)
+    git("commit", "-m", "succeeds")
+
+    assert "test.ipynb" in git("ls-files")
+    assert "test.py" in git("ls-files")
diff --git a/tests/utils.py b/tests/utils.py
index 1443862b..a8f41ca1 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -63,6 +63,9 @@ requires_myst = pytest.mark.skipif(
     not is_myst_available(), reason="myst_parser not found"
 )
 requires_no_myst = pytest.mark.skipif(is_myst_available(), reason="myst is available")
+requires_pre_commit = pytest.mark.skipif(
+    not tool_version("pre-commit"), reason="pre-commit not found"
+)
 skip_on_windows = pytest.mark.skipif(sys.platform.startswith("win"), reason="Issue 489")