From 20d7fbf994d94d21293d70a3bda25cc741e96091 Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Thu, 6 Jan 2022 00:35:06 +0100 Subject: [PATCH 1/9] Try to reproduce #900 with a test --- tests/test_pre_commit_1_sync_with_config.py | 110 ++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 tests/test_pre_commit_1_sync_with_config.py diff --git a/tests/test_pre_commit_1_sync_with_config.py b/tests/test_pre_commit_1_sync_with_config.py new file mode 100644 index 000000000..3e6a30a1e --- /dev/null +++ b/tests/test_pre_commit_1_sync_with_config.py @@ -0,0 +1,110 @@ +import pytest +from git.exc import HookExecutionError +from nbformat.v4.nbbase import new_markdown_cell +from pre_commit.main import main as pre_commit + +from jupytext import TextFileContentsManager, read, write + +from .utils import ( + skip_pre_commit_tests_on_windows, + skip_pre_commit_tests_when_jupytext_folder_is_not_a_git_repo, +) + + +@skip_pre_commit_tests_on_windows +@skip_pre_commit_tests_when_jupytext_folder_is_not_a_git_repo +def test_pre_commit_hook_sync_with_config( + tmpdir, + cwd_tmpdir, + tmp_repo, + jupytext_repo_root, + jupytext_repo_rev, + python_notebook, +): + pre_commit_config_yaml = f""" +repos: +- repo: {jupytext_repo_root} + rev: {jupytext_repo_rev} + hooks: + - id: jupytext + args: [--sync] +""" + tmpdir.join(".pre-commit-config.yaml").write(pre_commit_config_yaml) + + tmp_repo.git.add(".pre-commit-config.yaml") + pre_commit(["install", "--install-hooks", "-f"]) + + tmpdir.join(".jupytext.toml").write('formats = "ipynb,py:percent"\n') + + # create a test notebook and save it in Jupyter + nb = python_notebook + cm = TextFileContentsManager() + cm.root_dir = str(tmpdir) + cm.save(dict(type="notebook", content=nb), "test.ipynb") + + # Assert that "text_representation" is in the Jupytext metadata #900 + assert "text_representation" in tmpdir.join("test.py").read() + # But not in the ipynb notebook + assert "text_representation" not in tmpdir.join("test.ipynb").read() + + # try to commit it, should fail as the py version hasn't been added + tmp_repo.git.add("test.ipynb") + with pytest.raises( + HookExecutionError, + match="git add test.py", + ): + tmp_repo.index.commit("failing") + + # add the py file, now the commit will succeed + tmp_repo.git.add("test.py") + tmp_repo.index.commit("passing") + assert "test.ipynb" in tmp_repo.tree() + assert "test.py" in tmp_repo.tree() + + # The text representation metadata is still in the py file + assert "text_representation" in tmpdir.join("test.py").read() + # But not in the ipynb notebook + assert "text_representation" not in tmpdir.join("test.ipynb").read() + + # modify the ipynb file in another editor + nb = read("test.ipynb") + nb.cells.append(new_markdown_cell("A new cell")) + write(nb, "test.ipynb") + + tmp_repo.git.add("test.ipynb") + + # We try to commit one more time, this updates the py file + with pytest.raises( + HookExecutionError, + match="files were modified by this hook", + ): + tmp_repo.index.commit("failing") + + # the text file has been updated + assert "A new cell" in tmpdir.join("test.py").read() + + # trying to commit should fail again because we forgot to add the .py version + with pytest.raises(HookExecutionError, match="git add test.py"): + tmp_repo.index.commit("still failing") + + nb = read("test.ipynb") + assert len(nb.cells) == 2 + + # add the text file, now the commit will succeed + tmp_repo.git.add("test.py") + tmp_repo.index.commit("passing") + + # modify the .py file + nb.cells.append(new_markdown_cell("A third cell")) + write(nb, "test.py", fmt="py:percent") + tmp_repo.git.add("test.py") + + # the pre-commit hook will update the .ipynb file + with pytest.raises(HookExecutionError, match="git add test.ipynb"): + tmp_repo.index.commit("failing") + + tmp_repo.git.add("test.ipynb") + tmp_repo.index.commit("passing") + + nb = read("test.ipynb") + assert len(nb.cells) == 3 From 8e2f3bb84fc11da0aa3771ffc54a89ef2dd29498 Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 00:02:06 +0100 Subject: [PATCH 2/9] The problematic metadata appears when the notebook is edited --- tests/test_pre_commit_1_sync_with_config.py | 46 ++++++--------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/tests/test_pre_commit_1_sync_with_config.py b/tests/test_pre_commit_1_sync_with_config.py index 3e6a30a1e..17dd9380f 100644 --- a/tests/test_pre_commit_1_sync_with_config.py +++ b/tests/test_pre_commit_1_sync_with_config.py @@ -3,7 +3,7 @@ from nbformat.v4.nbbase import new_markdown_cell from pre_commit.main import main as pre_commit -from jupytext import TextFileContentsManager, read, write +from jupytext import TextFileContentsManager, read from .utils import ( skip_pre_commit_tests_on_windows, @@ -66,45 +66,23 @@ def test_pre_commit_hook_sync_with_config( # But not in the ipynb notebook assert "text_representation" not in tmpdir.join("test.ipynb").read() - # modify the ipynb file in another editor - nb = read("test.ipynb") + # modify the ipynb file in Jupyter + # Reload the notebook + nb = cm.get("test.ipynb")["content"] nb.cells.append(new_markdown_cell("A new cell")) - write(nb, "test.ipynb") - - tmp_repo.git.add("test.ipynb") + cm.save(dict(type="notebook", content=nb), "test.ipynb") - # We try to commit one more time, this updates the py file - with pytest.raises( - HookExecutionError, - match="files were modified by this hook", - ): - tmp_repo.index.commit("failing") + # The text representation metadata is in the py file + assert "text_representation" in tmpdir.join("test.py").read() + # But not in the ipynb notebook + assert "text_representation" not in tmpdir.join("test.ipynb").read() - # the text file has been updated + # the text file has been updated (and the ipynb file as well) assert "A new cell" in tmpdir.join("test.py").read() - - # trying to commit should fail again because we forgot to add the .py version - with pytest.raises(HookExecutionError, match="git add test.py"): - tmp_repo.index.commit("still failing") - nb = read("test.ipynb") assert len(nb.cells) == 2 - # add the text file, now the commit will succeed - tmp_repo.git.add("test.py") - tmp_repo.index.commit("passing") - - # modify the .py file - nb.cells.append(new_markdown_cell("A third cell")) - write(nb, "test.py", fmt="py:percent") - tmp_repo.git.add("test.py") - - # the pre-commit hook will update the .ipynb file - with pytest.raises(HookExecutionError, match="git add test.ipynb"): - tmp_repo.index.commit("failing") - + # add both files, the commit will succeed tmp_repo.git.add("test.ipynb") + tmp_repo.git.add("test.py") tmp_repo.index.commit("passing") - - nb = read("test.ipynb") - assert len(nb.cells) == 3 From 293d5f4a497fce87b6873ee61b1abee58a218db1 Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 01:18:08 +0100 Subject: [PATCH 3/9] Add a simpler test that reproduces #900 --- tests/test_cm_config.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test_cm_config.py b/tests/test_cm_config.py index 7bbb48e0c..732cdbc99 100644 --- a/tests/test_cm_config.py +++ b/tests/test_cm_config.py @@ -8,6 +8,7 @@ from tornado.web import HTTPError import jupytext +from jupytext import TextFileContentsManager from jupytext.compare import compare_cells from .utils import notebook_model @@ -192,3 +193,32 @@ def test_metadata_filter_from_config_has_precedence_over_notebook_metadata( py = tmpdir.join("test.py").read() assert "notebook_metadata_filter: all" in py + + +def test_test_no_text_representation_metadata_in_ipynb_900( + tmpdir, + python_notebook, +): + tmpdir.join(".jupytext.toml").write('formats = "ipynb,py:percent"\n') + + # create a test notebook and save it in Jupyter + nb = python_notebook + cm = TextFileContentsManager() + cm.root_dir = str(tmpdir) + cm.save(dict(type="notebook", content=nb), "test.ipynb") + + # Assert that "text_representation" is in the Jupytext metadata #900 + assert "text_representation" in tmpdir.join("test.py").read() + # But not in the ipynb notebook + assert "text_representation" not in tmpdir.join("test.ipynb").read() + + # modify the ipynb file in Jupyter + # Reload the notebook + nb = cm.get("test.ipynb")["content"] + nb.cells.append(new_markdown_cell("A new cell")) + cm.save(dict(type="notebook", content=nb), "test.ipynb") + + # The text representation metadata is in the py file + assert "text_representation" in tmpdir.join("test.py").read() + # But not in the ipynb notebook + assert "text_representation" not in tmpdir.join("test.ipynb").read() From 5abdeb7207ed4722c83d473e70156cf20c15d6c4 Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 01:33:11 +0100 Subject: [PATCH 4/9] Version 1.13.6-dev --- docs/CHANGELOG.md | 7 +++++++ jupytext/version.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 5f837d666..bca22a2d4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,13 @@ Jupytext ChangeLog ================== +1.13.5 (2021-12-27) +------------------- + +**Fixed** +- The `text_representation` metadata of text notebooks is filtered from `.ipynb` files both in `jupytext.write` and in the contents manager for Jupyter ([#900](https://github.com/mwouts/jupytext/issues/900)) + + 1.13.5 (2021-12-27) ------------------- diff --git a/jupytext/version.py b/jupytext/version.py index 21b3553b4..8c6f52528 100644 --- a/jupytext/version.py +++ b/jupytext/version.py @@ -1,3 +1,3 @@ """Jupytext's version number""" -__version__ = "1.13.5" +__version__ = "1.13.6-dev" From 0c94844637e5ec79eb5612e00dc7d2e899e00b5c Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 01:25:56 +0100 Subject: [PATCH 5/9] Drop the text_information metadata in ipynb files both in jupytext.write and in the contents manager --- jupytext/contentsmanager.py | 11 ++++++++++- jupytext/jupytext.py | 34 ++++++++++++++++++++++------------ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/jupytext/contentsmanager.py b/jupytext/contentsmanager.py index b4d114fb1..fc216da0d 100644 --- a/jupytext/contentsmanager.py +++ b/jupytext/contentsmanager.py @@ -31,6 +31,7 @@ short_form_multiple_formats, short_form_one_format, ) +from .jupytext import drop_text_representation_metadata from .kernels import set_kernelspec_from_language from .paired_paths import ( InconsistentPath, @@ -140,7 +141,15 @@ def save_one_file(path, fmt): self.create_prefix_dir(path, fmt) if fmt["extension"] == ".ipynb": - return self.super.save(model, path) + return self.super.save( + dict( + type="notebook", + content=drop_text_representation_metadata( + model["content"] + ), + ), + path, + ) if ( model["content"]["metadata"] diff --git a/jupytext/jupytext.py b/jupytext/jupytext.py index 2248f826e..40a382264 100644 --- a/jupytext/jupytext.py +++ b/jupytext/jupytext.py @@ -454,20 +454,9 @@ def writes(notebook, fmt, version=nbformat.NO_CONVERT, config=None, **kwargs): ext = fmt["extension"] format_name = fmt.get("format_name") - jupytext_metadata = metadata.get("jupytext", {}) - if ext == ".ipynb": - # Remove jupytext section if empty - jupytext_metadata.pop("text_representation", {}) - if not jupytext_metadata: - metadata.pop("jupytext", {}) return nbformat.writes( - NotebookNode( - nbformat=notebook.nbformat, - nbformat_minor=notebook.nbformat_minor, - metadata=metadata, - cells=notebook.cells, - ), + drop_text_representation_metadata(notebook, metadata), version, **kwargs, ) @@ -483,6 +472,27 @@ def writes(notebook, fmt, version=nbformat.NO_CONVERT, config=None, **kwargs): return writer.writes(notebook, metadata) +def drop_text_representation_metadata(notebook, metadata=None): + """When the notebook is saved to an ipynb file, we drop the text_representation metadata""" + if metadata is None: + # Make a copy to avoid modification by reference + metadata = deepcopy(notebook["metadata"]) + + jupytext_metadata = metadata.get("jupytext", {}) + jupytext_metadata.pop("text_representation", {}) + + # Remove the jupytext section if empty + if not jupytext_metadata: + metadata.pop("jupytext", {}) + + return NotebookNode( + nbformat=notebook["nbformat"], + nbformat_minor=notebook["nbformat_minor"], + metadata=metadata, + cells=notebook["cells"], + ) + + def write(nb, fp, version=nbformat.NO_CONVERT, fmt=None, config=None, **kwargs): """Write a notebook to a file name or a file object From b7e96d7e270f1755fea688ff5cc2cc6172487c1c Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 02:02:41 +0100 Subject: [PATCH 6/9] Suffix may start with '.', '-' or '_' --- docs/CHANGELOG.md | 5 ++++- jupytext/cli.py | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index bca22a2d4..c8f4c3f0d 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,12 +1,15 @@ Jupytext ChangeLog ================== -1.13.5 (2021-12-27) +1.13.6 (2022-01-11) ------------------- **Fixed** - The `text_representation` metadata of text notebooks is filtered from `.ipynb` files both in `jupytext.write` and in the contents manager for Jupyter ([#900](https://github.com/mwouts/jupytext/issues/900)) +**Changed** +- Jupytext will not issue a warning when a format suffix starting with '.', '-' or '_' is passed to the `--to` option ([#901](https://github.com/mwouts/jupytext/issues/901)) + 1.13.5 (2021-12-27) ------------------- diff --git a/jupytext/cli.py b/jupytext/cli.py index e7e757034..d6cb6a46a 100644 --- a/jupytext/cli.py +++ b/jupytext/cli.py @@ -452,7 +452,8 @@ def fmt_if_not_ipynb(nb): not args.output and args.output_format and "." in args.output_format - and not args.output_format.startswith(".") + # a suffix is expected to start with one of these characters #901 + and not args.output_format.startswith((".", "-", "_")) and "//" not in args.output_format ): @@ -461,7 +462,7 @@ def single_line(msg, *args, **kwargs): warnings.formatwarning = single_line warnings.warn( - "You have passed a file name to the '--to' option, " + "You might have passed a file name to the '--to' option, " "when a format description was expected. Maybe you want to use the '-o' option instead?" ) From c76cc73a532bf85656fded16370aca10c5bd322f Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 02:02:55 +0100 Subject: [PATCH 7/9] Update LICENSE years --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index de45b5caa..546201584 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2018-2021 Marc Wouts +Copyright (c) 2018-2022 Marc Wouts Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal From 136586589d0c7b4f03a3641188feb1aea308e331 Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 02:03:20 +0100 Subject: [PATCH 8/9] Version 1.13.6 --- jupytext/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupytext/version.py b/jupytext/version.py index 8c6f52528..6ee113872 100644 --- a/jupytext/version.py +++ b/jupytext/version.py @@ -1,3 +1,3 @@ """Jupytext's version number""" -__version__ = "1.13.6-dev" +__version__ = "1.13.6" From cfc9e1c993c91d2e6db6da65b1ce58bfb8757d34 Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 08:46:41 +0100 Subject: [PATCH 9/9] Fix LGTM alert Module 'jupytext' is imported with both 'import' and 'import from' --- jupytext/contentsmanager.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/jupytext/contentsmanager.py b/jupytext/contentsmanager.py index fc216da0d..09045a36a 100644 --- a/jupytext/contentsmanager.py +++ b/jupytext/contentsmanager.py @@ -14,8 +14,6 @@ except ImportError: pass -import jupytext - from .config import ( JUPYTEXT_CONFIG_FILES, PYPROJECT_FILE, @@ -31,7 +29,7 @@ short_form_multiple_formats, short_form_one_format, ) -from .jupytext import drop_text_representation_metadata +from .jupytext import drop_text_representation_metadata, reads, writes from .kernels import set_kernelspec_from_language from .paired_paths import ( InconsistentPath, @@ -167,7 +165,7 @@ def save_one_file(path, fmt): text_model = dict( type="file", format="text", - content=jupytext.writes( + content=writes( nbformat.from_dict(model["content"]), fmt=fmt, config=config ), ) @@ -220,7 +218,7 @@ def get( model["format"] = "json" model["mimetype"] = None try: - model["content"] = jupytext.reads( + model["content"] = reads( model["content"], fmt=fmt, config=config ) except Exception as err: @@ -303,7 +301,7 @@ def read_one_file(alt_path, alt_fmt): text = self.super.get( alt_path, content=True, type="file", format=format )["content"] - return jupytext.reads(text, fmt=alt_fmt, config=config) + return reads(text, fmt=alt_fmt, config=config) inputs, outputs = latest_inputs_and_outputs( path, fmt, formats, get_timestamp, contents_manager_mode=True