From f8e8352859cc22e17b11154d0770fd946c4a430a Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Tue, 11 Jan 2022 09:23:50 +0100 Subject: [PATCH] Filter the text_representation metadata in the contents manager (#903) * Drop the text_information metadata in ipynb files both in jupytext.write and in the contents manager * Format suffix may start with '.', '-' or '_' --- LICENSE | 2 +- docs/CHANGELOG.md | 10 +++ jupytext/cli.py | 5 +- jupytext/contentsmanager.py | 19 +++-- jupytext/jupytext.py | 34 +++++--- jupytext/version.py | 2 +- tests/test_cm_config.py | 30 +++++++ tests/test_pre_commit_1_sync_with_config.py | 88 +++++++++++++++++++++ 8 files changed, 168 insertions(+), 22 deletions(-) create mode 100644 tests/test_pre_commit_1_sync_with_config.py 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 diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 5f837d666..c8f4c3f0d 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,16 @@ Jupytext ChangeLog ================== +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?" ) diff --git a/jupytext/contentsmanager.py b/jupytext/contentsmanager.py index b4d114fb1..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,6 +29,7 @@ short_form_multiple_formats, short_form_one_format, ) +from .jupytext import drop_text_representation_metadata, reads, writes from .kernels import set_kernelspec_from_language from .paired_paths import ( InconsistentPath, @@ -140,7 +139,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"] @@ -158,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 ), ) @@ -211,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: @@ -294,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 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 diff --git a/jupytext/version.py b/jupytext/version.py index 21b3553b4..6ee113872 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" 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() 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..17dd9380f --- /dev/null +++ b/tests/test_pre_commit_1_sync_with_config.py @@ -0,0 +1,88 @@ +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 + +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 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() + + # the text file has been updated (and the ipynb file as well) + assert "A new cell" in tmpdir.join("test.py").read() + nb = read("test.ipynb") + assert len(nb.cells) == 2 + + # add both files, the commit will succeed + tmp_repo.git.add("test.ipynb") + tmp_repo.git.add("test.py") + tmp_repo.index.commit("passing")