Skip to content

Commit

Permalink
Filter the text_representation metadata in the contents manager (#903)
Browse files Browse the repository at this point in the history
* Drop the text_information metadata in ipynb files
both in jupytext.write and in the contents manager
* Format suffix may start with '.', '-' or '_'
  • Loading branch information
mwouts authored Jan 11, 2022
1 parent 57d83aa commit f8e8352
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 22 deletions.
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 10 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
-------------------

Expand Down
5 changes: 3 additions & 2 deletions jupytext/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
):

Expand All @@ -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?"
)

Expand Down
19 changes: 13 additions & 6 deletions jupytext/contentsmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
except ImportError:
pass

import jupytext

from .config import (
JUPYTEXT_CONFIG_FILES,
PYPROJECT_FILE,
Expand All @@ -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,
Expand Down Expand Up @@ -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"]
Expand All @@ -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
),
)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
34 changes: 22 additions & 12 deletions jupytext/jupytext.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion jupytext/version.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""Jupytext's version number"""

__version__ = "1.13.5"
__version__ = "1.13.6"
30 changes: 30 additions & 0 deletions tests/test_cm_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
88 changes: 88 additions & 0 deletions tests/test_pre_commit_1_sync_with_config.py
Original file line number Diff line number Diff line change
@@ -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")

0 comments on commit f8e8352

Please sign in to comment.