[PRECOMMIT] JuPyText does not preserve ids during pre-commit hook bidirectional sync.

[Skylion007]

I recently tried to upgrade habitat_sim to use the new pre-commit hook, but found that the ID is not preserved in the cells with a bidirectional update.

pre-commit hook settings

-   repo: https://github.com/mwouts/jupytext/
    rev: v1.11.1
    hooks:
    -   id: jupytext
        files: '^examples/tutorials/(colabs|nb_python)/(.*\.py|.*\.ipynb)$'
        args: [--update-metadata, '{"jupytext":{"notebook_metadata_filter":"all", "cell_metadata_filter":"-all"}, "accelerator":"GPU"}', --set-formats, 'nb_python//py:percent,colabs//ipynb,', --pipe, black, --pipe, 'isort - --treat-comment-as-code "# %%"', --pipe-fmt, 'py:percent', --sync]
        additional_dependencies:
            - black
            - isort

I also tried this simplified version here:

-   repo: https://github.com/mwouts/jupytext/
    rev: v1.11.1
    hooks:
    -   id: jupytext
        files: '^examples/tutorials/nb_python/.*\.py$'
        args: [--update-metadata, '{"jupytext":{"notebook_metadata_filter":"all", "cell_metadata_filter":"-all"}, "accelerator":"GPU"}', --set-formats, 'nb_python//py:percent,colabs//ipynb,', --pipe, black, --pipe, 'isort - --treat-comment-as-code "# %%"', --pipe-fmt, 'py:percent', --sync]
        additional_dependencies:
            - black
            - isort

but ran into the same problem where it would keep updating the id every time pre-commit is run.

[Skylion007]

Got around it for now with the old "'nbformat<=5.0.8'", but that's not a long term fix obviously.

[mwouts]

Hi @Skylion007 , thanks for letting me know. My expectation would have been that the ids, although not in the text representation, should be preserved (like outputs and metadata) when doing either --sync or --to --update. I'll have a look.

[mwouts]

Hi @Skylion007 , I'll soon have a look at this issue, may I ask you what you mean by ID is not preserved in the cells with a bidirectional update ? Do you think you could reproduce the issue in the context of a test like e.g. tests.test_pre_commit_3_sync_black_nbstripout.test_pre_commit_hook_sync_black_nbstripout ? Thanks!

[Skylion007]

My best guess is it was this corner case.

• Update JuPyText hook so the version comment in all the .py files change.
• Update NBConvert so that the notebook format changes.
• These changes conflict so jupytext will try to update both. Doing this will change the ID for every run of JuPyText.
• Pre-Commit hook will never pass due to constantly changing ID I just tried to reproduce it though by only updating nbconvert and it didn't work, so I suspect it's when BOTH are updated at the same time. It was a pretty big version change though 1.6 -> to current so I don't know if it was a one off bug.

Basically it was happening before / after this PR: https://github.com/facebookresearch/habitat-sim/pull/1200 (with the nbconvert constraint removed).

[mwouts]

Update JuPyText hook so the version comment in all the .py files change.

Oh interesting! I've seen people using

notebook_metadata_filter: -jupytext.text_representation.jupytext_version,-jupytext.text_representation.format_version

to avoid this impact on their text notebooks (yet I am not sure I would recommend that because then I can't provide version compatibility)

Let me know if there is anything else I can do to help.