Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement require_hash in the get method of Jupytext's content manager #1202

Merged
merged 2 commits into from
Dec 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/step_tests-pip.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ jobs:
${{ matrix.no_kernel && '(No kernel)' }}
${{ matrix.markdown-it-py && format('(markdown-it-py {0})', matrix.markdown-it-py) }}
${{ matrix.no_markdown-it-py && '(No markdown-it-py)'}}
${{ matrix.jupyter_server && '(Jupyer-server {0})'}}
runs-on: ubuntu-latest

strategy:
Expand All @@ -29,6 +30,9 @@ jobs:
python-version: [ "3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]
experimental: [false]
include:
# Test with jupyter-server=2.10 that does not have the 'require_hash' argument
- python-version: "3.x"
jupyter_server: "2.10.0"
# Test minimum markdown-it-py supported (otherwise the most recent version is used)
- python-version: "3.x"
markdown-it-py: "~=2.0"
Expand Down Expand Up @@ -58,6 +62,10 @@ jobs:
jupyterlab
${{ format('markdown-it-py{0}', matrix.markdown-it-py) }}

- name: Install Jupyter Server
if: ${{ matrix.jupyter_server }}
run: python -m pip install 'jupyter_server~=${{ matrix.jupyter_server }}'

- name: List the versions of the Jupyter components
run: jupyter --version

Expand Down
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
Jupytext ChangeLog
==================

1.16.5-dev
----------
1.16.5 (2024-12-15)
-------------------

**Fixed**
- We have fixed a notebook corruption issue when using Jupytext with Jupyter-Collaboration ([#1124](https://github.com/mwouts/jupytext/issues/1124), [jupyter-collaboration #214](https://github.com/jupyterlab/jupyter-collaboration/issues/214)).
- We have fixed a notebook corruption issue when using Jupytext with Jupyter-Collaboration ([#1124](https://github.com/mwouts/jupytext/issues/1124), [jupyter-collaboration 214](https://github.com/mwouts/jupytext/issues/214)).
- We have added the `require_hash` argument on the Jupytext contents manager. The hash of a paired file is the concatenation of the hash of the text file and the hash for the `.ipynb` file ([#1165](https://github.com/mwouts/jupytext/issues/1165))
- The `rst2md` tests have been fixed by requiring `sphinx<8` ([#1266](https://github.com/mwouts/jupytext/issues/1266))
- Some dependencies of the JupyterLab extensions were updated ([#1272](https://github.com/mwouts/jupytext/issues/1272), [#1273](https://github.com/mwouts/jupytext/issues/1273), [#1280](https://github.com/mwouts/jupytext/issues/1280), [#1285](https://github.com/mwouts/jupytext/issues/1285), [#1290](https://github.com/mwouts/jupytext/issues/1290))
- The pre-commit hook is now compatible with log.showsignature=True ([#1281](https://github.com/mwouts/jupytext/issues/1281)). Thanks to [Justin Lecher](https://github.com/jlec) for this fix.
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
[![CI](https://github.com/mwouts/jupytext/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/mwouts/jupytext/actions)
[![Documentation Status](https://readthedocs.org/projects/jupytext/badge/?version=latest)](https://jupytext.readthedocs.io/en/latest/?badge=latest)
[![codecov.io](https://codecov.io/github/mwouts/jupytext/coverage.svg?branch=main)](https://codecov.io/gh/mwouts/jupytext/branch/main)
[![MIT License](https://img.shields.io/github/license/mwouts/jupytext)](LICENSE)
[![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)
[![GitHub language count](https://img.shields.io/github/languages/count/mwouts/jupytext)](docs/languages.md)
[![Conda Version](https://img.shields.io/conda/vn/conda-forge/jupytext.svg)](https://anaconda.org/conda-forge/jupytext)
Expand Down
60 changes: 55 additions & 5 deletions src/jupytext/contentsmanager.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""ContentsManager that allows to open Rmd, py, R and ipynb files as notebooks
"""
import inspect
import itertools
import os

Expand Down Expand Up @@ -184,35 +185,60 @@
self.log.error("Error while saving file: %s %s", path, e, exc_info=True)
raise HTTPError(500, f"Unexpected error while saving file: {path} {e}")

def get(
def _get_with_no_require_hash_argument(
self,
path,
content=True,
type=None,
format=None,
load_alternative_format=True,
):
return self._get_with_require_hash_argument(
path,
content=content,
type=type,
format=format,
require_hash=False,
load_alternative_format=load_alternative_format,
)

def _get_with_require_hash_argument(
self,
path,
content=True,
type=None,
format=None,
require_hash=False,
load_alternative_format=True,
):
"""Takes a path for an entity and returns its model"""
path = path.strip("/")
ext = os.path.splitext(path)[1]

super_kwargs = {"content": content, "type": type, "format": format}
if require_hash:
super_kwargs["require_hash"] = require_hash

# Not a notebook?
if (
not self.file_exists(path)
or self.dir_exists(path)
or (type is not None and type != "notebook")
):
return self.super.get(path, content, type, format)
return self.super.get(path, **super_kwargs)

config = self.get_config(path, use_cache=content is False)
if ext not in self.all_nb_extensions(config):
return self.super.get(path, content, type, format)
return self.super.get(path, **super_kwargs)

fmt = preferred_format(ext, config.preferred_jupytext_formats_read)
if ext == ".ipynb":
model = self.super.get(path, content, type="notebook", format=format)
super_kwargs["type"] = "notebook"
model = self.super.get(path, **super_kwargs)
else:
model = self.super.get(path, content, type="file", format="text")
super_kwargs["type"] = "file"
super_kwargs["format"] = "text"
model = self.super.get(path, **super_kwargs)
model["type"] = "notebook"
if content:
# We may need to update these keys, inherited from text files formats
Expand Down Expand Up @@ -314,6 +340,21 @@
# Modification time of a paired notebook is the timestamp of inputs #118 #978
model["last_modified"] = inputs.timestamp

if require_hash:
if inputs.path is None or outputs.path is None:
return model

Check warning on line 345 in src/jupytext/contentsmanager.py

View check run for this annotation

Codecov / codecov/patch

src/jupytext/contentsmanager.py#L345

Added line #L345 was not covered by tests
model_other = self.super.get(
inputs.path if path == outputs.path else outputs.path,
require_hash=True,
)
# The hash of a paired file is the concatenation of
# the hashes of the input and output files
if path == outputs.path:
model["hash"] = model_other["hash"] + model["hash"]
else:
model["hash"] = model["hash"] + model_other["hash"]
return model

if not content:
return model

Expand Down Expand Up @@ -612,6 +653,15 @@
self.notebook_extensions = self.notebook_extensions.split(",")
return self

if "require_hash" in inspect.signature(base_contents_manager_class.get).parameters:
JupytextContentsManager.get = (
JupytextContentsManager._get_with_require_hash_argument
)
else:
JupytextContentsManager.get = (
JupytextContentsManager._get_with_no_require_hash_argument
)

return JupytextContentsManager


Expand Down
2 changes: 1 addition & 1 deletion src/jupytext/version.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""Jupytext's version number"""

__version__ = "1.16.5-dev"
__version__ = "1.16.5"
42 changes: 42 additions & 0 deletions tests/integration/contents_manager/test_contentsmanager.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import inspect
import os
import re
import shutil
Expand Down Expand Up @@ -1851,3 +1852,44 @@ def test_move_paired_notebook_to_subdir_1059(tmp_path, python_notebook):
model = cm.get("scripts/subdir/my_notebook.py")
nb = model["content"]
compare_notebooks(nb, python_notebook, fmt="py:percent")


def test_hash_changes_if_paired_file_is_edited(tmp_path, python_notebook):
# 1. write py ipynb
cm = jupytext.TextFileContentsManager()

if "require_hash" not in inspect.signature(cm.get).parameters:
pytest.skip(
reason="This JupytextContentsManager does not have a 'require_hash' parameter in cm.get"
)

cm.formats = "ipynb,py:percent"
cm.root_dir = str(tmp_path)

# save ipynb
nb = python_notebook
nb_name = "notebook.ipynb"
cm.save(model=notebook_model(nb), path=nb_name)
org_model = cm.get(nb_name, require_hash=True)

py_file = tmp_path / "notebook.py"

text = py_file.read_text()
assert "# %% [markdown]" in text.splitlines(), text

# modify the timestamp of the paired file
time.sleep(0.5)
py_file.write_text(text)
model = cm.get(nb_name, require_hash=True)
# not sure why the hash changes on Windows?
assert (model["hash"] == org_model["hash"]) or (os.name == "nt")

# modify the paired file
py_file.write_text(text + "\n# %%\n1 + 1\n")

new_model = cm.get(nb_name, require_hash=True)
assert new_model["hash"] != org_model["hash"]

# the hash is for the pair (inputs first)
model_from_py_file = cm.get("notebook.py", require_hash=True)
assert model_from_py_file["hash"] == new_model["hash"]
Loading