Repository: mwouts/jupytext

Issue:
Jupytext manager seems to fail with jupyterfs
The content manager for jupytext seems to want a call _get_os_path which seems to break with jupyter-fs metamanager 
see  jpmorganchase/jupyter-fs#53


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/mwouts/jupytext/pull/621?src=pr&el=h1) Report
> Merging [#621](https://codecov.io/gh/mwouts/jupytext/pull/621?src=pr&el=desc) into [master](https://codecov.io/gh/mwouts/jupytext/commit/b72d03f39920333eb312d675a1ecd0fa7c2b549f?el=desc) will **decrease** coverage by `0.27%`.
> The diff coverage is `25.71%`.

[![Impacted file tree graph](https://codecov.io/gh/mwouts/jupytext/pull/621/graphs/tree.svg?width=650&height=150&src=pr&token=a1pNjsiILi)](https://codecov.io/gh/mwouts/jupytext/pull/621?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #621      +/-   ##
==========================================
- Coverage   99.17%   98.89%   -0.28%     
==========================================
  Files          94       94              
  Lines        9199     9229      +30     
==========================================
+ Hits         9123     9127       +4     
- Misses         76      102      +26     
```


| [Impacted Files](https://codecov.io/gh/mwouts/jupytext/pull/621?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tests/test\_contentsmanager.py](https://codecov.io/gh/mwouts/jupytext/pull/621/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9jb250ZW50c21hbmFnZXIucHk=) | `97.72% <12.00%> (-2.28%)` | :arrow_down: |
| [jupytext/contentsmanager.py](https://codecov.io/gh/mwouts/jupytext/pull/621/diff?src=pr&el=tree#diff-anVweXRleHQvY29udGVudHNtYW5hZ2VyLnB5) | `96.17% <60.00%> (-1.66%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mwouts/jupytext/pull/621?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/mwouts/jupytext/pull/621?src=pr&el=footer). Last update [b72d03f...d0d8668](https://codecov.io/gh/mwouts/jupytext/pull/621?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/.github/workflows/continuous-integration-pip.yml b/.github/workflows/continuous-integration-pip.yml
index b7f2d02..3437ae2 100644
--- a/.github/workflows/continuous-integration-pip.yml
+++ b/.github/workflows/continuous-integration-pip.yml
@@ -25,6 +25,7 @@ jobs:
           pip install autopep8 || true
           # install sphinx_gallery and matplotlib if available (may not work on pypy)
           pip install sphinx_gallery~=0.7.0 || true
+          pip install jupyter-fs || true
       - name: Lint with flake8
         run: |
           # stop the build if there are Python syntax errors or undefined names
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4ded9ce..70bc6e6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,7 +6,7 @@
 
 **Fixed**
 - Freeze optional dependency on `sphinx-gallery` to version `~=0.7.0` (#614)
-
+- Jupytext's contents manager can be used with `jupyterfs` (#618)
 
 1.6.0 (2020-09-01)
 ------------------
diff --git a/jupytext/config.py b/jupytext/config.py
index b50a0a7..b77aff2 100644
--- a/jupytext/config.py
+++ b/jupytext/config.py
@@ -1,9 +1,10 @@
 """Find and read Jupytext configuration files"""
 import os
 import yaml
+import json
 from traitlets import Unicode, Float, Bool, Enum
 from traitlets.config import Configurable
-from traitlets.config.loader import JSONFileConfigLoader, PyFileConfigLoader
+from traitlets.config.loader import PyFileConfigLoader
 from traitlets.traitlets import TraitError
 from .formats import (
     NOTEBOOK_EXTENSIONS,
@@ -290,25 +291,25 @@ def find_jupytext_configuration_file(path, search_parent_dirs=True):
     return find_jupytext_configuration_file(parent_dir)
 
 
-def load_jupytext_configuration_file(jupytext_config_file):
-    """Read a Jupytext config file, and return a JupytextConfig object"""
+def load_jupytext_configuration_file(jupytext_config_file, stream=None):
+    """Read a Jupytext config file, and return a dict"""
+    if not jupytext_config_file.endswith(".py") and stream is None:
+        with open(jupytext_config_file) as stream:
+            return load_jupytext_configuration_file(jupytext_config_file, stream.read())
 
     try:
         if jupytext_config_file.endswith((".toml", "jupytext")):
             import toml
 
-            config = toml.load(jupytext_config_file)
-            return config
+            return toml.loads(stream)
 
         if jupytext_config_file.endswith((".yml", ".yaml")):
-            with open(jupytext_config_file) as stream:
-                config = yaml.safe_load(stream)
-            return config
+            return yaml.safe_load(stream)
 
-        if jupytext_config_file.endswith(".py"):
-            return PyFileConfigLoader(jupytext_config_file).load_config()
+        if jupytext_config_file.endswith(".json"):
+            return json.loads(stream)
 
-        return JSONFileConfigLoader(jupytext_config_file).load_config()
+        return PyFileConfigLoader(jupytext_config_file).load_config()
     except (ValueError, NameError) as err:
         raise JupytextConfigurationError(
             "The Jupytext configuration file {} is incorrect: {}".format(
@@ -324,16 +325,24 @@ def load_jupytext_config(nb_file):
         return None
     if os.path.isfile(nb_file) and os.path.samefile(config_file, nb_file):
         return None
-    config_values = load_jupytext_configuration_file(config_file)
+    config_file = find_jupytext_configuration_file(nb_file)
+    config_dict = load_jupytext_configuration_file(config_file)
+    return validate_jupytext_configuration_file(config_file, config_dict)
+
+
+def validate_jupytext_configuration_file(config_file, config_dict):
+    """Turn a dict-like config into a JupytextConfiguration object"""
+    if config_dict is None:
+        return None
     try:
-        config = JupytextConfiguration(**config_values)
+        config = JupytextConfiguration(**config_dict)
     except TraitError as err:
         raise JupytextConfigurationError(
             "The Jupytext configuration file {} is incorrect: {}".format(
                 config_file, err
             )
         )
-    invalid_options = set(config_values).difference(dir(JupytextConfiguration()))
+    invalid_options = set(config_dict).difference(dir(JupytextConfiguration()))
     if any(invalid_options):
         raise JupytextConfigurationError(
             "The Jupytext configuration file {} is incorrect: options {} are not supported".format(
diff --git a/jupytext/contentsmanager.py b/jupytext/contentsmanager.py
index 0eb3d87..c29a5dc 100644
--- a/jupytext/contentsmanager.py
+++ b/jupytext/contentsmanager.py
@@ -19,7 +19,6 @@ except ImportError:
     pass
 
 from .jupytext import reads, writes
-from .jupytext import create_prefix_dir as create_prefix_dir_from_path
 from .formats import long_form_multiple_formats
 from .formats import short_form_one_format, short_form_multiple_formats
 from .paired_paths import (
@@ -34,8 +33,11 @@ from .kernels import set_kernelspec_from_language
 from .config import (
     JupytextConfiguration,
     JupytextConfigurationError,
+    JUPYTEXT_CONFIG_FILES,
+    find_global_jupytext_configuration_file,
+    load_jupytext_configuration_file,
+    validate_jupytext_configuration_file,
     preferred_format,
-    load_jupytext_config,
     prepare_notebook_for_save,
 )
 
@@ -64,7 +66,7 @@ def build_jupytext_contents_manager_class(base_contents_manager_class):
         Python (.py) or R scripts (.R)
         """
 
-        def __init__(self, **kwargs):
+        def __init__(self, *args, **kwargs):
             # Dictionary: notebook path => (fmt, formats) where
             # fmt is the current format, and formats the paired formats.
             self.paired_notebooks = dict()
@@ -72,7 +74,7 @@ def build_jupytext_contents_manager_class(base_contents_manager_class):
             # Configuration cache, useful when notebooks are listed in a given directory
             self.cached_config = namedtuple("cached_config", "path timestamp config")
 
-            super(JupytextContentsManager, self).__init__(**kwargs)
+            super(JupytextContentsManager, self).__init__(*args, **kwargs)
 
         def all_nb_extensions(self):
             """All extensions that should be classified as notebooks"""
@@ -116,7 +118,12 @@ def build_jupytext_contents_manager_class(base_contents_manager_class):
 
         def create_prefix_dir(self, path, fmt):
             """Create the prefix dir, if missing"""
-            create_prefix_dir_from_path(self._get_os_path(path.strip("/")), fmt)
+            if "prefix" in fmt and "/" in path:
+                parent_dir = self.get_parent_dir(path)
+                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)
 
         def save(self, model, path=""):
             """Save the file model and return the model with no content."""
@@ -180,14 +187,12 @@ def build_jupytext_contents_manager_class(base_contents_manager_class):
         ):
             """ Takes a path for an entity and returns its model"""
             path = path.strip("/")
-
-            os_path = self._get_os_path(path)
             ext = os.path.splitext(path)[1]
 
             # Not a notebook?
             if (
-                not self.exists(path)
-                or os.path.isdir(os_path)
+                not self.file_exists(path)
+                or self.dir_exists(path)
                 or (type != "notebook" if type else ext not in self.all_nb_extensions())
             ):
                 return super(JupytextContentsManager, self).get(
@@ -422,10 +427,44 @@ def build_jupytext_contents_manager_class(base_contents_manager_class):
             self.drop_paired_notebook(old_path)
             self.update_paired_notebooks(new_path, formats)
 
+        def get_parent_dir(self, path):
+            """The parent directory"""
+            if "/" in path:
+                return path.rsplit("/", 1)[0]
+            return ""
+
+        def get_config_file(self, directory):
+            """Return the jupytext configuration file, if any"""
+            for jupytext_config_file in JUPYTEXT_CONFIG_FILES:
+                path = directory + "/" + jupytext_config_file
+                if self.file_exists(path):
+                    return path
+
+            if not directory:
+                return find_global_jupytext_configuration_file()
+
+            parent_dir = self.get_parent_dir(directory)
+            return self.get_config_file(parent_dir)
+
+        def load_config_file(self, config_file):
+            """Load the configuration file"""
+            if config_file is None:
+                return None
+            self.log.info("Loading Jupytext configuration file at %s", config_file)
+            if config_file.endswith(".py"):
+                config_dict = load_jupytext_configuration_file(
+                    self._get_os_path(config_file)
+                )
+            else:
+                model = self.get(config_file, content=True, type="file")
+                config_dict = load_jupytext_configuration_file(
+                    config_file, model["content"]
+                )
+            return validate_jupytext_configuration_file(config_file, config_dict)
+
         def get_config(self, path, use_cache=False):
             """Return the Jupytext configuration for the given path"""
-            nb_file = self._get_os_path(path.strip("/"))
-            parent_dir = os.path.dirname(nb_file)
+            parent_dir = self.get_parent_dir(path)
 
             # When listing the notebooks for the tree view, we use a one-second
             # cache for the configuration file
@@ -437,9 +476,10 @@ def build_jupytext_contents_manager_class(base_contents_manager_class):
                 )
             ):
                 try:
+                    config_file = self.get_config_file(parent_dir)
+                    self.cached_config.config = self.load_config_file(config_file)
                     self.cached_config.path = parent_dir
                     self.cached_config.timestamp = datetime.now()
-                    self.cached_config.config = load_jupytext_config(parent_dir)
                 except JupytextConfigurationError as err:
                     raise HTTPError(400, "{}".format(err))
 
diff --git a/jupytext/formats.py b/jupytext/formats.py
index e1693fd..0078c5a 100644
--- a/jupytext/formats.py
+++ b/jupytext/formats.py
@@ -5,8 +5,8 @@ new formats here!
 
 import os
 import re
-import yaml
 import warnings
+import yaml
 import nbformat
 from .header import header_to_metadata_and_cell, insert_or_test_version_number
 from .cell_reader import (
diff --git a/jupytext/header.py b/jupytext/header.py
index 438c889..412a9df 100644
--- a/jupytext/header.py
+++ b/jupytext/header.py
@@ -209,11 +209,10 @@ def header_to_metadata_and_cell(
             if not started:
                 started = True
                 continue
-            else:
-                ended = True
-                if in_html_div:
-                    continue
-                break
+            ended = True
+            if in_html_div:
+                continue
+            break
 
         if _JUPYTER_RE.match(line):
             in_jupyter = True
diff --git a/jupytext/pandoc.py b/jupytext/pandoc.py
index 335a1c4..3d69f64 100644
--- a/jupytext/pandoc.py
+++ b/jupytext/pandoc.py
@@ -44,6 +44,7 @@ def is_pandoc_available():
 
 
 def raise_if_pandoc_is_not_available():
+    """Raise with an informative error message if pandoc is not available"""
     try:
         from pkg_resources import parse_version
     except ImportError:


Test patch:
diff --git a/tests/test_cm_config.py b/tests/test_cm_config.py
index ccfcdd2..06a0c20 100644
--- a/tests/test_cm_config.py
+++ b/tests/test_cm_config.py
@@ -36,6 +36,7 @@ def test_local_config_overrides_cm_config(tmpdir):
 def test_config_file_is_called_just_once(tmpdir, n=2):
     cm = jupytext.TextFileContentsManager()
     cm.root_dir = str(tmpdir)
+    tmpdir.join("jupytext.toml").write("")
     nb_files = [str(tmpdir.join("notebook{}.ipynb".format(i))) for i in range(n)]
 
     for nb_file in nb_files:
@@ -43,7 +44,9 @@ def test_config_file_is_called_just_once(tmpdir, n=2):
 
     mock_config = mock.MagicMock(return_value=None)
 
-    with mock.patch("jupytext.contentsmanager.load_jupytext_config", mock_config):
+    with mock.patch(
+        "jupytext.contentsmanager.load_jupytext_configuration_file", mock_config
+    ):
         for i in range(n):
             cm.get("notebook{}.ipynb".format(i), content=False)
 
@@ -85,18 +88,13 @@ def test_incorrect_config_message(tmpdir, cfg_file, cfg_text):
     cm = jupytext.TextFileContentsManager()
     cm.root_dir = str(tmpdir)
 
-    cfg_file = tmpdir.join(cfg_file)
-    cfg_file.write(cfg_text)
-
+    tmpdir.join(cfg_file).write(cfg_text)
     tmpdir.join("empty.ipynb").write("{}")
 
-    expected_message = "The Jupytext configuration file {} is incorrect".format(
+    expected_message = "The Jupytext configuration file .*{} is incorrect".format(
         cfg_file
     )
 
-    # This is a regexp so we have to escape the path separator on Windows
-    expected_message = expected_message.replace("\\", "\\\\")
-
     with pytest.raises(HTTPError, match=expected_message):
         cm.get("empty.ipynb", type="notebook", content=False)
 
diff --git a/tests/test_contentsmanager.py b/tests/test_contentsmanager.py
index 8e70a13..24fda0c 100644
--- a/tests/test_contentsmanager.py
+++ b/tests/test_contentsmanager.py
@@ -548,7 +548,7 @@ def test_save_using_preferred_and_default_format_170(nb_file, tmpdir):
     cm.preferred_jupytext_formats_save = "py:percent"
     cm.default_jupytext_formats = "ipynb,python//py"
 
-    # save to ipynb and oy
+    # save to ipynb and py
     cm.save(model=dict(type="notebook", content=nb), path="notebook.ipynb")
 
     # read py file
@@ -563,7 +563,7 @@ def test_save_using_preferred_and_default_format_170(nb_file, tmpdir):
     cm.preferred_jupytext_formats_save = "python//py:percent"
     cm.default_jupytext_formats = "ipynb,python//py"
 
-    # save to ipynb and oy
+    # save to ipynb and py
     cm.save(model=dict(type="notebook", content=nb), path="notebook.ipynb")
 
     # read py file
@@ -1715,3 +1715,100 @@ def test_new_untitled(tmpdir):
     assert cm.new_untitled(type="notebook", ext=".py")["path"] == "Untitled2.py"
     assert cm.new_untitled(type="notebook")["path"] == "Untitled3.ipynb"
     assert cm.new_untitled(type="notebook", ext=".py:percent")["path"] == "Untitled4.py"
+
+
+def test_nested_prefix(tmpdir):
+    cm = jupytext.TextFileContentsManager()
+    cm.root_dir = str(tmpdir)
+
+    # save to ipynb and py
+    nb = new_notebook(
+        cells=[new_code_cell("1+1"), new_markdown_cell("Some text")],
+        metadata={"jupytext": {"formats": "ipynb,nested/prefix//.py"}},
+    )
+    cm.save(model=dict(type="notebook", content=nb), path="notebook.ipynb")
+
+    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"""
+    try:
+        from jupyterfs.fsmanager import FSManager
+    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 = ""
+
+    # save a few files
+    text = "some text\n"
+    cm.save(dict(type="file", content=text, format="text"), path="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")
+
+    # list the directory
+    directory = cm.get("/")
+    assert set(file["name"] for file in directory["content"]) == {
+        "text.md",
+        "text_notebook.md",
+        "notebook.ipynb",
+    }
+
+    # get the files
+    model = cm.get("/text.md", type="file")
+    assert model["type"] == "file"
+    assert model["content"] == text
+
+    model = cm.get("/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)
+        assert model["type"] == "notebook"
+        actual_cells = model["content"].cells
+
+        # saving adds 'trusted=True' to the code cell metadata
+        for cell in actual_cells:
+            cell.metadata = {}
+        compare(actual_cells, nb.cells)
+
+
+def test_config_jupytext_jupyter_fs_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 = ""
+
+    # 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")
+
+    # list the directory
+    directory = cm.get("/")
+    assert set(file["name"] for file in directory["content"]) == {
+        "jupytext.toml",
+        "text.md",
+        "text_notebook.md",
+        "notebook.ipynb",
+        "notebook.py",
+        "script.py",
+        "script.ipynb",
+    }