Repository: mwouts/jupytext

Issue:
Running isort on Jupytext notebooks represented as scripts break some cells
This is related to #446 -  It would be great to be able to run `isort` easily on a Jupytext notebook, however ATM it does not work that well.

Indeed, `isort` seems to move the comments preceding an `import` together with the import. Thus, when we run `isort` on a `.py` notebook, imports at the top of the cell are moved along the preceding cell marker (`# %%`  or `# +`), breaking the corresponding cell.

Possibly one way to fix this is to insert one blank line between the cell marker and the cell content. I'll see if we can add an option to do that, and if `isort` works better, then. Does this option work well with `black`? If so, it should be made the default for when a notebook is piped into an other command.


Discussion:

Patch:
diff --git a/.github/workflows/continuous-integration-conda.yml b/.github/workflows/continuous-integration-conda.yml
index f779772..68d356b 100644
--- a/.github/workflows/continuous-integration-conda.yml
+++ b/.github/workflows/continuous-integration-conda.yml
@@ -60,6 +60,8 @@ jobs:
           conda install mock --freeze-installed
           # install black if available (Python 3.6 and above), and autopep8 for testing the pipe mode
           conda install black --freeze-installed
+          # install isort from source
+          pip install git+https://github.com/timothycrosley/isort.git
           conda install autopep8 --freeze-installed
           # install sphinx_gallery and matplotlib if available
           conda install sphinx-gallery --freeze-installed
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f5772b1..48398a6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
 **Added**
 - Activated GitHub code scanning alerts
 - New option `hide_notebook_metadata` to encapsulate the notebook metadata in an HTML comment (#527)
+- Tested `isort` on notebooks (#553)
 
 **Changed**
 - Install Jupytext from source on MyBinder to avoid cache issues (#567)
@@ -13,6 +14,7 @@
 **Fixed**
 - Configured coverage targets in `codecov.yml`
 - Only scripts can have an encoding comment, not Markdown or R Markdown files (#576)
+- Support spaces in `--pipe` commands (#562)
 
 
 1.5.2 (2020-07-21)
diff --git a/docs/using-cli.md b/docs/using-cli.md
index a327726..f284515 100644
--- a/docs/using-cli.md
+++ b/docs/using-cli.md
@@ -43,6 +43,12 @@ You may also find useful to `--pipe` the text representation of a notebook into
 jupytext --sync --pipe black notebook.ipynb    # read most recent version of notebook, reformat with black, save
 ```
 
+To reorder the imports in your notebook, use
+```bash
+jupytext --pipe 'isort - --treat-comment-as-code "# %%" --float-to-top' notebook.ipynb
+```
+(remove the `--float-to-top` argument if you prefer to run `isort` per cell).
+
 For programs that don't accept pipes, use `{}` as a placeholder for the name of a temporary file that will contain the text representation of the notebook. For instance, run `pytest` on your notebook with:
 ```bash
 jupytext --check 'pytest {}' notebook.ipynb    # export the notebook in format py:percent in a temp file, run pytest
diff --git a/environment.yml b/environment.yml
index 870c940..5b2cc3a 100644
--- a/environment.yml
+++ b/environment.yml
@@ -17,6 +17,7 @@ dependencies:
   - pylint
   - flake8
   - black
+  - isort
   - autopep8
   - sphinx-gallery
   - nodejs<13
diff --git a/jupytext/cli.py b/jupytext/cli.py
index 5c8b7c5..97edd0a 100644
--- a/jupytext/cli.py
+++ b/jupytext/cli.py
@@ -4,6 +4,7 @@ import os
 import re
 import sys
 import glob
+import shlex
 import subprocess
 import argparse
 import json
@@ -774,8 +775,7 @@ def load_paired_notebook(notebook, fmt, nb_file, log):
 
 def exec_command(command, input=None, capture=False):
     """Execute the desired command, and pipe the given input into it"""
-    if not isinstance(command, list):
-        command = command.split(" ")
+    assert isinstance(command, list)
     sys.stdout.write("[jupytext] Executing {}\n".format(" ".join(command)))
     process = subprocess.Popen(
         command,
@@ -816,7 +816,7 @@ def pipe_notebook(notebook, command, fmt="py:percent", update=True, prefix=None)
     fmt = check_auto_ext(fmt, notebook.metadata, "--pipe-fmt")
     text = writes(notebook, fmt)
 
-    command = command.split(" ")
+    command = shlex.split(command)
     if "{}" in command:
         if prefix is not None:
             prefix = prefix + (" " if " " in prefix else "_")


Test patch:
diff --git a/tests/test_isort.py b/tests/test_isort.py
new file mode 100644
index 0000000..0c03ca1
--- /dev/null
+++ b/tests/test_isort.py
@@ -0,0 +1,43 @@
+from jupytext import reads, writes
+from jupytext.cli import pipe_notebook
+from jupytext.compare import compare
+
+from .utils import requires_isort
+
+
+@requires_isort
+def test_pipe_into_isort():
+    text_org = """# %%
+import numpy as np
+np.array([1,2,3])
+
+# %%
+import pandas as pd
+pd.Series([1,2,3])
+
+# %%
+# This is a comment on the second import
+import pandas as pd
+pd.Series([4,5,6])
+"""
+
+    text_target = """# %%
+import numpy as np
+# This is a comment on the second import
+import pandas as pd
+
+np.array([1,2,3])
+
+# %%
+pd.Series([1,2,3])
+
+# %%
+pd.Series([4,5,6])
+"""
+
+    nb_org = reads(text_org, fmt="py:percent")
+    nb_pipe = pipe_notebook(
+        nb_org, 'isort - --treat-comment-as-code "# %%" --float-to-top'
+    )
+    text_actual = writes(nb_pipe, "py:percent")
+    compare(text_actual, text_target)
diff --git a/tests/test_using_cli.py b/tests/test_using_cli.py
index 4d40041..47caa17 100644
--- a/tests/test_using_cli.py
+++ b/tests/test_using_cli.py
@@ -1,5 +1,4 @@
 import os
-import re
 import shlex
 import pytest
 import jupytext
@@ -36,12 +35,23 @@ def test_jupytext_commands_in_the_documentation_work(tmpdir):
         for cmd in cell.source.splitlines():
             if not cmd.startswith("jupytext"):
                 continue
+
+            # Do not test commands that involve reading a notebook from stdin
             if "read ipynb from stdin" in cmd:
                 continue
+
+            # We can't run pytest inside pytest
             if "pytest {}" in cmd:
                 continue
+
+            # We need to remove the comments that may follow the jupytext command
+            if "#" in cmd:
+                left, comment = cmd.rsplit("#", 1)
+                if '"' not in comment:
+                    cmd = left
+
             print("Testing: {}".format(cmd))
-            args = shlex.split(re.sub(r"#.*", "", cmd))[1:]
+            args = shlex.split(cmd)[1:]
             assert not jupytext_cli(args), cmd
             cmd_tested += 1
 
diff --git a/tests/utils.py b/tests/utils.py
index 60c9535..c6c98f4 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -27,6 +27,7 @@ requires_jupytext_installed = pytest.mark.skipif(
     not tool_version("jupytext"), reason="jupytext is not installed"
 )
 requires_black = pytest.mark.skipif(not tool_version("black"), reason="black not found")
+requires_isort = pytest.mark.skipif(not tool_version("isort"), reason="isort not found")
 requires_flake8 = pytest.mark.skipif(
     not tool_version("flake8"), reason="flake8 not found"
 )