Skip to content

Commit

Permalink
Jupytext formats use /, while paths might use either / or \
Browse files Browse the repository at this point in the history
Reproduce and fix an `InconsistentPath` issue with notebooks paired with scripts in a folder
  • Loading branch information
mwouts committed Jul 12, 2021
1 parent 76ce21c commit 66418e8
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 35 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Jupytext ChangeLog
- (Documentation) the `cell_markers` option (and the other ones) can be set directly in the `jupytext.toml` config file ([#809](https://github.com/mwouts/jupytext/issues/809)).

**Fixed**
- The prefix in the Jupytext formats always use /, while paths might use either / or \ ([#806](https://github.com/mwouts/jupytext/issues/806))
- Fixed an `InconsistentPath` issue with notebooks paired with scripts in a folder ([#806](https://github.com/mwouts/jupytext/issues/806))
- The pre-commit tests are skipped when the Jupytext folder is not a git repository ([#814](https://github.com/mwouts/jupytext/issues/814))


Expand Down
76 changes: 47 additions & 29 deletions jupytext/paired_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@ class InconsistentPath(ValueError):
information it contains"""


def split(path):
"""Split the current path in parent path / current directory or file, using either / or the local path separator"""
if "/" not in path and os.path.sep not in path:
def split(path, sep):
if sep not in path:
return "", path

if path.rfind("/") < path.rfind(os.path.sep):
return path.rsplit(os.path.sep, 1)
return path.rsplit(sep, 1)

return path.rsplit("/", 1)

def join(left, right, sep):
if left:
return left + sep + right
return right


def separator(path):
Expand All @@ -35,31 +37,47 @@ def separator(path):
return "/"


def base_path(main_path, fmt):
def base_path(main_path, fmt, formats=None):
"""Given a path and options for a format (ext, suffix, prefix), return the corresponding base path"""
if not fmt or (isinstance(fmt, dict) and "extension" not in fmt):
base, ext = os.path.splitext(main_path)
fmt = long_form_one_format(fmt)

base, ext = os.path.splitext(main_path)
if "extension" not in fmt:
fmt["extension"] = ext
if ext not in NOTEBOOK_EXTENSIONS:
raise InconsistentPath(
"'{}' is not a notebook. Supported extensions are '{}'.".format(
main_path, "', '".join(NOTEBOOK_EXTENSIONS)
)
)
return base

fmt = long_form_one_format(fmt)
fmt_ext = fmt["extension"]
suffix = fmt.get("suffix")
prefix = fmt.get("prefix")

base, ext = os.path.splitext(main_path)
if ext != fmt_ext:
if ext != fmt["extension"]:
raise InconsistentPath(
u"Notebook path '{}' was expected to have extension '{}'".format(
main_path, fmt_ext
main_path, fmt["extension"]
)
)

# Is there a format that matches the main path?
if formats is None:
formats = [fmt]

for f in formats:
if f["extension"] != fmt["extension"]:
continue
if (
"format_name" in fmt
and "format_name" in f
and f["format_name"] != fmt["format_name"]
):
continue
# extend 'fmt' with the format information (prefix, suffix) from f
fmt = {key: fmt.get(key, value) for key, value in f.items()}
break

suffix = fmt.get("suffix")
prefix = fmt.get("prefix")

if suffix:
if not base.endswith(suffix):
raise InconsistentPath(
Expand All @@ -76,9 +94,9 @@ def base_path(main_path, fmt):
prefix_root, prefix = prefix.rsplit("//", 1)
else:
prefix_root = ""
prefix_dir, prefix_file_name = split(prefix)
notebook_dir, notebook_file_name = split(base)
sep = separator(base)
notebook_dir, notebook_file_name = split(base, sep)
prefix_dir, prefix_file_name = split(prefix, "/")

base_dir = None
config_file = find_jupytext_configuration_file(notebook_dir)
Expand All @@ -102,19 +120,19 @@ def base_path(main_path, fmt):
parent_prefix_dir = prefix_dir
actual_folders = list()
while parent_prefix_dir:
parent_prefix_dir, expected_folder = os.path.split(parent_prefix_dir)
parent_prefix_dir, expected_folder = split(parent_prefix_dir, "/")
if expected_folder == "..":
if not actual_folders:
raise InconsistentPath(
u"Notebook directory '{}' does not match prefix '{}'".format(
notebook_dir, prefix_dir
)
)
parent_notebook_dir = os.path.join(
parent_notebook_dir, actual_folders.pop()
parent_notebook_dir = join(
parent_notebook_dir, actual_folders.pop(), sep
)
else:
parent_notebook_dir, actual_folder = os.path.split(parent_notebook_dir)
parent_notebook_dir, actual_folder = split(parent_notebook_dir, sep)
actual_folders.append(actual_folder)

if actual_folder != expected_folder:
Expand Down Expand Up @@ -164,7 +182,7 @@ def full_path(base, fmt):
prefix_root, prefix = prefix.rsplit("//", 1)
else:
prefix_root = ""
prefix_dir, prefix_file_name = split(prefix)
prefix_dir, prefix_file_name = split(prefix, "/")

# Local path separator (\\ on windows)
sep = separator(base)
Expand All @@ -179,10 +197,10 @@ def full_path(base, fmt):
)
if prefix_root:
left, right = base.rsplit("//", 1)
right_dir, notebook_file_name = split(right)
right_dir, notebook_file_name = split(right, sep)
notebook_dir = left + prefix_root + sep + right_dir
else:
notebook_dir, notebook_file_name = split(base)
notebook_dir, notebook_file_name = split(base, sep)

if prefix_file_name:
notebook_file_name = prefix_file_name + notebook_file_name
Expand All @@ -191,7 +209,7 @@ def full_path(base, fmt):
dotdot = ".." + sep
while prefix_dir.startswith(dotdot):
prefix_dir = prefix_dir[len(dotdot) :]
notebook_dir = os.path.dirname(notebook_dir)
notebook_dir = split(notebook_dir, sep)[0]

# Do not add a path separator when notebook_dir is '/'
if notebook_dir and not notebook_dir.endswith(sep):
Expand Down Expand Up @@ -235,7 +253,7 @@ def paired_paths(main_path, fmt, formats):
formats = long_form_multiple_formats(formats)

# Is there a format that matches the main path?
base = base_path(main_path, fmt)
base = base_path(main_path, fmt, formats)
paths = [full_path(base, f) for f in formats]

if main_path not in paths:
Expand Down
3 changes: 1 addition & 2 deletions tests/test_black.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,8 @@ def test_apply_black_through_jupytext(tmpdir, nb_file):
compare_notebooks(nb_now, nb_black, compare_ids=True)

# Write to another folder using dots
script_fmt = os.path.join("..", "script_folder//py:percent")
write(nb_org, tmp_ipynb)
jupytext([tmp_ipynb, "--to", script_fmt, "--pipe", "black"])
jupytext([tmp_ipynb, "--to", "../script_folder//py:percent", "--pipe", "black"])
assert os.path.isfile(tmp_py)
nb_now = read(tmp_py)
nb_now.metadata = metadata
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ def test_format_prefix_suffix(tmpdir, cwd_tmpdir):
tmp_py = "scripts/notebook_name.py"
write(new_notebook(), tmp_ipynb)

jupytext([tmp_ipynb, "--to", os.path.join("..", "scripts//py")])
jupytext([tmp_ipynb, "--to", "../scripts//py"])
assert os.path.isfile(tmp_py)
os.remove(tmp_py)

Expand Down
71 changes: 68 additions & 3 deletions tests/test_paired_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

import pytest

import jupytext
from jupytext.cli import jupytext as jupytext_cli
from jupytext.compare import compare
from jupytext.contentsmanager import TextFileContentsManager
from jupytext.formats import (
Expand Down Expand Up @@ -103,10 +105,19 @@ def test_paired_paths_windows_no_subfolder():
paired_paths(nb_file, "notebooks///ipynb", formats)


def test_paired_path_dotdot_564():
main_path = "examples/tutorials/colabs/rigid_object_tutorial.ipynb"
@pytest.mark.parametrize("os_path_sep", ["\\", "/"])
def test_paired_path_dotdot_564(os_path_sep):
main_path = os_path_sep.join(
["examples", "tutorials", "colabs", "rigid_object_tutorial.ipynb"]
)
formats = "../nb_python//py:percent,../colabs//ipynb"
paired_paths(main_path, "ipynb", formats)
with mock.patch("os.path.sep", os_path_sep):
assert base_path(
main_path, None, long_form_multiple_formats(formats)
) == os_path_sep.join(
["examples", "tutorials", "colabs", "rigid_object_tutorial"]
)
paired_paths(main_path, "ipynb", formats)


def test_path_in_tree_limited_to_config_dir(tmpdir):
Expand Down Expand Up @@ -247,3 +258,57 @@ def test_cm_paired_paths():
zero = ""
cm.update_paired_notebooks("nb.ipynb", zero)
assert cm.paired_notebooks == {}


def test_paired_path_with_prefix(
nb_file="scripts/test.py",
fmt={"extension": ".py", "format_name": "percent"},
formats=[
{"extension": ".ipynb"},
{"prefix": "scripts/", "format_name": "percent", "extension": ".py"},
],
):
assert paired_paths(nb_file, fmt, formats) == [
("test.ipynb", {"extension": ".ipynb"}),
(
"scripts/test.py",
{"prefix": "scripts/", "format_name": "percent", "extension": ".py"},
),
]


def test_paired_notebook_ipynb_root_scripts_in_folder_806(
tmpdir, cwd_tmpdir, python_notebook
):
"""In this test we pair a notebook with a script in a subfolder, and then do some
natural operations like delete/recreate one of the paired files"""
# Save sample notebook
test_ipynb = tmpdir / "test.ipynb"
jupytext.write(python_notebook, str(test_ipynb))

# Pair the notebook to a script in a subfolder
jupytext_cli(["--set-formats", "ipynb,scripts//py:percent", "test.ipynb"])
assert (tmpdir / "scripts" / "test.py").exists()

# Delete and then recreate the ipynb notebook using --to notebook
test_ipynb.remove()
jupytext_cli(
[
"--to",
"notebook",
"--output",
"test.ipynb",
"scripts/test.py",
]
)
assert test_ipynb.exists()

# Delete and then recreate the ipynb notebook using --sync
test_ipynb.remove()
jupytext_cli(
[
"--sync",
"scripts/test.py",
]
)
assert test_ipynb.exists()

0 comments on commit 66418e8

Please sign in to comment.