From 044e02da11e44ebeccf6fbc189fceb1f6809ffdd Mon Sep 17 00:00:00 2001 From: Marc Wouts Date: Wed, 23 Sep 2020 00:00:34 +0200 Subject: [PATCH] Reproduce & fix the error 'JupytextContentsManager' object has no attribute '_notebook_model' --- jupytext/contentsmanager.py | 112 ++++++++++++++++++---------------- setup.cfg | 2 +- tests/test_contentsmanager.py | 65 +++++++++++--------- 3 files changed, 98 insertions(+), 81 deletions(-) diff --git a/jupytext/contentsmanager.py b/jupytext/contentsmanager.py index c29a5dc31..165cc5a42 100644 --- a/jupytext/contentsmanager.py +++ b/jupytext/contentsmanager.py @@ -5,11 +5,6 @@ from datetime import timedelta, datetime from collections import namedtuple import nbformat - -try: - import unittest.mock as mock -except ImportError: - import mock from tornado.web import HTTPError # import notebook.transutils before notebook.services.contents.filemanager #75 @@ -18,7 +13,7 @@ except ImportError: pass -from .jupytext import reads, writes +import jupytext from .formats import long_form_multiple_formats from .formats import short_form_one_format, short_form_multiple_formats from .paired_paths import ( @@ -42,20 +37,6 @@ ) -def _jupytext_writes(fmt): - def _writes(nbk, version=nbformat.NO_CONVERT, **kwargs): - return writes(nbk, fmt, version=version, **kwargs) - - return _writes - - -def _jupytext_reads(fmt): - def _reads(text, as_version, **kwargs): - return reads(text, fmt, as_version=as_version, **kwargs) - - return _reads - - def build_jupytext_contents_manager_class(base_contents_manager_class): """Derives a TextFileContentsManager class from the given base class""" @@ -72,9 +53,11 @@ def __init__(self, *args, **kwargs): self.paired_notebooks = dict() # Configuration cache, useful when notebooks are listed in a given directory - self.cached_config = namedtuple("cached_config", "path timestamp config") - - super(JupytextContentsManager, self).__init__(*args, **kwargs) + self.cached_config = namedtuple( + "cached_config", "path timestamp config_file config" + ) + self.super = super(JupytextContentsManager, self) + self.super.__init__(*args, **kwargs) def all_nb_extensions(self): """All extensions that should be classified as notebooks""" @@ -123,12 +106,12 @@ def create_prefix_dir(self, path, fmt): if not self.dir_exists(parent_dir): self.create_prefix_dir(parent_dir, fmt) self.log.info("Creating directory %s", parent_dir) - self.save(dict(type="directory"), parent_dir) + self.super.save(dict(type="directory"), parent_dir) def save(self, model, path=""): """Save the file model and return the model with no content.""" if model["type"] != "notebook": - return super(JupytextContentsManager, self).save(model, path) + return self.super.save(model, path) path = path.strip("/") nbk = model["content"] @@ -154,7 +137,7 @@ def save_one_file(path, fmt): self.create_prefix_dir(path, fmt) if fmt["extension"] == ".ipynb": - return super(JupytextContentsManager, self).save(model, path) + return self.super.save(model, path) if ( model["content"]["metadata"] @@ -169,8 +152,13 @@ def save_one_file(path, fmt): ) ) - with mock.patch("nbformat.writes", _jupytext_writes(fmt)): - return super(JupytextContentsManager, self).save(model, path) + text_model = dict( + type="file", + format="text", + content=jupytext.writes(model["content"], fmt=fmt), + ) + + return self.super.save(text_model, path) return write_pair(path, jupytext_formats, save_one_file) @@ -195,18 +183,21 @@ def get( or self.dir_exists(path) or (type != "notebook" if type else ext not in self.all_nb_extensions()) ): - return super(JupytextContentsManager, self).get( - path, content, type, format - ) + return self.super.get(path, content, type, format) config = self.get_config(path, use_cache=content is False) fmt = preferred_format(ext, config.preferred_jupytext_formats_read) if ext == ".ipynb": - model = self._notebook_model(path, content=content) + model = self.super.get(path, content, type="notebook", format=format) else: + model = self.super.get(path, content, type="file", format=format) + model["type"] = "notebook" config.set_default_format_options(fmt, read=True) - with mock.patch("nbformat.reads", _jupytext_reads(fmt)): - model = self._notebook_model(path, content=content) + if content: + try: + model["content"] = jupytext.reads(model["content"], fmt=fmt) + except Exception as err: + raise HTTPError(400, str(err)) if not load_alternative_format: return model @@ -220,7 +211,7 @@ def get( fmt, formats = self.paired_notebooks.get(path) for alt_path, _ in paired_paths(path, fmt, formats): if alt_path != path and self.exists(alt_path): - alt_model = self._notebook_model(alt_path, content=False) + alt_model = self.super.get(alt_path, content=False) if alt_model["last_modified"] > model["last_modified"]: model["last_modified"] = alt_model["last_modified"] @@ -262,19 +253,23 @@ def get_timestamp(alt_path): return None if alt_path == path: return model["last_modified"] - return self._notebook_model(alt_path, content=False)["last_modified"] + return self.super.get(alt_path, content=False)["last_modified"] def read_one_file(alt_path, alt_fmt): if alt_path == path: return model["content"] if alt_path.endswith(".ipynb"): self.log.info(u"Reading OUTPUTS from {}".format(alt_path)) - return self._notebook_model(alt_path, content=True)["content"] + return self.super.get( + alt_path, content=True, type="notebook", format=format + )["content"] self.log.info(u"Reading SOURCE from {}".format(alt_path)) config.set_default_format_options(alt_fmt, read=True) - with mock.patch("nbformat.reads", _jupytext_reads(alt_fmt)): - return self._notebook_model(alt_path, content=True)["content"] + text = self.super.get( + alt_path, content=True, type="file", format=format + )["content"] + return jupytext.reads(text, fmt=alt_fmt) inputs, outputs = latest_inputs_and_outputs( path, fmt, formats, get_timestamp, contents_manager_mode=True @@ -335,9 +330,7 @@ def new_untitled(self, path="", type="", ext=""): into account when type=="notebook". See https://github.com/mwouts/jupytext/issues/443 """ if type != "notebook" and ext != ".ipynb": - return super(JupytextContentsManager, self).new_untitled( - path, type, ext - ) + return self.super.new_untitled(path, type, ext) ext = ext or ".ipynb" if ":" in ext: @@ -386,13 +379,13 @@ def increment_notebook_filename(self, filename, path=""): def trust_notebook(self, path): """Trust the current notebook""" if path.endswith(".ipynb") or path not in self.paired_notebooks: - super(JupytextContentsManager, self).trust_notebook(path) + self.super.trust_notebook(path) return fmt, formats = self.paired_notebooks[path] for alt_path, alt_fmt in paired_paths(path, fmt, formats): if alt_fmt["extension"] == ".ipynb": - super(JupytextContentsManager, self).trust_notebook(alt_path) + self.super.trust_notebook(alt_path) def rename_file(self, old_path, new_path): """Rename the current notebook, as well as its alternative representations""" @@ -405,7 +398,7 @@ def rename_file(self, old_path, new_path): pass if old_path not in self.paired_notebooks: - super(JupytextContentsManager, self).rename_file(old_path, new_path) + self.super.rename_file(old_path, new_path) return fmt, formats = self.paired_notebooks.get(old_path) @@ -420,9 +413,7 @@ def rename_file(self, old_path, new_path): for old_alt_path, alt_fmt in old_alt_paths: new_alt_path = full_path(new_base, alt_fmt) if self.exists(old_alt_path): - super(JupytextContentsManager, self).rename_file( - old_alt_path, new_alt_path - ) + self.super.rename_file(old_alt_path, new_alt_path) self.drop_paired_notebook(old_path) self.update_paired_notebooks(new_path, formats) @@ -431,6 +422,11 @@ def get_parent_dir(self, path): """The parent directory""" if "/" in path: return path.rsplit("/", 1)[0] + # jupyter-fs + if ":" in path and hasattr(self, "_managers"): + if path.endswith(":"): + return "" + return path.rsplit(":", 1)[0] + ":" return "" def get_config_file(self, directory): @@ -456,7 +452,7 @@ def load_config_file(self, config_file): self._get_os_path(config_file) ) else: - model = self.get(config_file, content=True, type="file") + model = self.super.get(config_file, content=True, type="file") config_dict = load_jupytext_configuration_file( config_file, model["content"] ) @@ -477,13 +473,27 @@ def get_config(self, path, use_cache=False): ): try: config_file = self.get_config_file(parent_dir) - self.cached_config.config = self.load_config_file(config_file) + if config_file: + self.cached_config.config = self.load_config_file(config_file) + else: + config_file = find_global_jupytext_configuration_file() + self.cached_config.config = self.load_config_file( + config_file, True + ) + self.cached_config.config_file = config_file self.cached_config.path = parent_dir self.cached_config.timestamp = datetime.now() except JupytextConfigurationError as err: raise HTTPError(400, "{}".format(err)) - return self.cached_config.config or self + if self.cached_config.config is not None: + self.log.debug( + "Configuration file for %s is %s", + path, + self.cached_config.config_file, + ) + return self.cached_config.config + return self return JupytextContentsManager diff --git a/setup.cfg b/setup.cfg index e769ae87c..848f32c75 100644 --- a/setup.cfg +++ b/setup.cfg @@ -18,4 +18,4 @@ exclude_lines = [flake8] max-line-length=127 exclude=tests/notebooks/* -ignore=E203,W503 +ignore=E203,E231,W503 diff --git a/tests/test_contentsmanager.py b/tests/test_contentsmanager.py index 24fda0c70..dba2f2ecf 100644 --- a/tests/test_contentsmanager.py +++ b/tests/test_contentsmanager.py @@ -5,6 +5,7 @@ import time import pytest import itertools +import logging import shutil from nbformat.v4.nbbase import new_notebook, new_markdown_cell, new_code_cell from tornado.web import HTTPError @@ -1731,29 +1732,41 @@ def test_nested_prefix(tmpdir): assert tmpdir.join("nested").join("prefix").join("notebook.py").isfile() -def test_jupytext_jupyter_fs_manager(tmpdir): - """Test the basic get/save functions of Jupytext with a fs manager - https://github.com/mwouts/jupytext/issues/618""" +def fs_meta_manager(tmpdir): try: - from jupyterfs.fsmanager import FSManager + from jupyterfs.metamanager import MetaManager except ImportError: pytest.skip("jupyterfs is not available") - cm_class = jupytext.build_jupytext_contents_manager_class(FSManager) - cm = cm_class("osfs://{local_dir}".format(local_dir=tmpdir)) - cm.default_jupytext_formats = "" + cm_class = jupytext.build_jupytext_contents_manager_class(MetaManager) + logger = logging.getLogger("jupyter-fs") + cm = cm_class(parent=None, log=logger) + cm.initResource( + { + "url": "osfs://{}".format(tmpdir), + } + ) + return cm + + +def test_jupytext_jupyter_fs_metamanager(tmpdir): + """Test the basic get/save functions of Jupytext with a fs manager + https://github.com/mwouts/jupytext/issues/618""" + cm = fs_meta_manager(tmpdir) + # the hash that corresponds to the osfs + osfs = [h for h in cm._managers if h != ""][0] # save a few files text = "some text\n" - cm.save(dict(type="file", content=text, format="text"), path="text.md") + cm.save(dict(type="file", content=text, format="text"), path=osfs + ":text.md") nb = new_notebook( cells=[new_markdown_cell("A markdown cell"), new_code_cell("1 + 1")] ) - cm.save(dict(type="notebook", content=nb), "notebook.ipynb") - cm.save(dict(type="notebook", content=nb), "text_notebook.md") + cm.save(dict(type="notebook", content=nb), osfs + ":notebook.ipynb") + cm.save(dict(type="notebook", content=nb), osfs + ":text_notebook.md") # list the directory - directory = cm.get("/") + directory = cm.get(osfs + ":/") assert set(file["name"] for file in directory["content"]) == { "text.md", "text_notebook.md", @@ -1761,17 +1774,17 @@ def test_jupytext_jupyter_fs_manager(tmpdir): } # get the files - model = cm.get("/text.md", type="file") + model = cm.get(osfs + ":/text.md", type="file") assert model["type"] == "file" assert model["content"] == text - model = cm.get("/text.md", type="notebook") + model = cm.get(osfs + ":text.md", type="notebook") assert model["type"] == "notebook" # We only compare the cells, as kernelspecs are added to the notebook metadata compare(model["content"].cells, [new_markdown_cell(text.strip())]) for nb_file in ["notebook.ipynb", "text_notebook.md"]: - model = cm.get(nb_file) + model = cm.get(osfs + ":" + nb_file) assert model["type"] == "notebook" actual_cells = model["content"].cells @@ -1781,28 +1794,22 @@ def test_jupytext_jupyter_fs_manager(tmpdir): compare(actual_cells, nb.cells) -def test_config_jupytext_jupyter_fs_manager(tmpdir): +def test_config_jupytext_jupyter_fs_meta_manager(tmpdir): """Test the configuration of Jupytext with a fs manager""" - try: - from jupyterfs.fsmanager import FSManager - except ImportError: - pytest.skip("jupyterfs is not available") - tmpdir.join("jupytext.toml").write('default_jupytext_formats = "ipynb,py"') - - cm_class = jupytext.build_jupytext_contents_manager_class(FSManager) - cm = cm_class("osfs://{local_dir}".format(local_dir=tmpdir)) - cm.default_jupytext_formats = "" + cm = fs_meta_manager(tmpdir) + # the hash that corresponds to the osfs + osfs = [h for h in cm._managers if h != ""][0] # save a few files nb = new_notebook() - cm.save(dict(type="file", content="text", format="text"), path="text.md") - cm.save(dict(type="notebook", content=nb), "script.py") - cm.save(dict(type="notebook", content=nb), "text_notebook.md") - cm.save(dict(type="notebook", content=nb), "notebook.ipynb") + cm.save(dict(type="file", content="text", format="text"), path=osfs + ":text.md") + cm.save(dict(type="notebook", content=nb), osfs + ":script.py") + cm.save(dict(type="notebook", content=nb), osfs + ":text_notebook.md") + cm.save(dict(type="notebook", content=nb), osfs + ":notebook.ipynb") # list the directory - directory = cm.get("/") + directory = cm.get(osfs + ":/") assert set(file["name"] for file in directory["content"]) == { "jupytext.toml", "text.md",