From e97f241aa38af774f2dbfbb2f7041bd3c5fdb35c Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Thu, 18 Nov 2021 21:10:15 -0600 Subject: [PATCH 01/12] Include cell magics in `validate_cell` Let `validate_cell()` also catch non-Python cell magics, as the bodies of these might contain invalid indentation (such as `%%writefile`) and this could cause TransformerManager to break. --- src/black/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index a5ddec91221..2ce3c272bba 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -57,6 +57,7 @@ remove_trailing_semicolon, put_trailing_semicolon_back, TRANSFORMED_MAGICS, + NON_PYTHON_CELL_MAGICS, jupyter_dependencies_are_installed, ) @@ -943,7 +944,7 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo def validate_cell(src: str) -> None: - """Check that cell does not already contain TransformerManager transformations. + """Check that cell does not already contain TransformerManager transformations, or cell magics, which might cause TransformerManager to break. If a cell contains ``!ls``, then it'll be transformed to ``get_ipython().system('ls')``. However, if the cell originally contained @@ -959,6 +960,8 @@ def validate_cell(src: str) -> None: """ if any(transformed_magic in src for transformed_magic in TRANSFORMED_MAGICS): raise NothingChanged + elif any('%%' + cell_magic in src for cell_magic in NON_PYTHON_CELL_MAGICS): + raise NothingChanged def format_cell(src: str, *, fast: bool, mode: Mode) -> str: From c0e9ef8a58761c173dee4a417f13bd3a2499ee76 Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 00:29:21 -0600 Subject: [PATCH 02/12] add test for "badly" indented text file this should be a valid arbitrary writefile, but before it failed via the tokenizer. --- tests/test_ipynb.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_ipynb.py b/tests/test_ipynb.py index ba460074e9a..de398ecbadf 100644 --- a/tests/test_ipynb.py +++ b/tests/test_ipynb.py @@ -102,6 +102,7 @@ def test_magic(src: str, expected: str) -> None: ( "%%bash\n2+2", "%%html --isolated\n2+2", + "%%writefile e.txt\n meh\n meh", ), ) def test_non_python_magics(src: str) -> None: From dbf3e556bc70df92ecab4f0a7277e5c8d057d5d3 Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 06:57:23 +0000 Subject: [PATCH 03/12] minor formatting --- src/black/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 2ce3c272bba..1bd11abd92f 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -944,7 +944,8 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo def validate_cell(src: str) -> None: - """Check that cell does not already contain TransformerManager transformations, or cell magics, which might cause TransformerManager to break. + """Check that cell does not already contain TransformerManager transformations, + or cell magics, which might cause TransformerManager to break. If a cell contains ``!ls``, then it'll be transformed to ``get_ipython().system('ls')``. However, if the cell originally contained @@ -960,8 +961,8 @@ def validate_cell(src: str) -> None: """ if any(transformed_magic in src for transformed_magic in TRANSFORMED_MAGICS): raise NothingChanged - elif any('%%' + cell_magic in src for cell_magic in NON_PYTHON_CELL_MAGICS): - raise NothingChanged + if any("%%" + cell_magic in src for cell_magic in NON_PYTHON_CELL_MAGICS): + raise NothingChanged def format_cell(src: str, *, fast: bool, mode: Mode) -> str: From 40d34313001a972597f9e825f3e8e281864f21cb Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 07:08:29 +0000 Subject: [PATCH 04/12] correct and extend docstring --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 1bd11abd92f..f951b697361 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -945,7 +945,7 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo def validate_cell(src: str) -> None: """Check that cell does not already contain TransformerManager transformations, - or cell magics, which might cause TransformerManager to break. + or cell magics, which might cause tokenizer_rt to break because of indentations. If a cell contains ``!ls``, then it'll be transformed to ``get_ipython().system('ls')``. However, if the cell originally contained From 80788ef96fae8507f13b64cf4b9b86d3c1a69da7 Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 07:15:02 +0000 Subject: [PATCH 05/12] add non-Python note and changelog --- CHANGES.md | 6 ++++++ src/black/__init__.py | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 35024de724d..ee4dcae410c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,11 @@ # Change Log +## Unreleased + +### _Black_ + +- Fixed non-Python cell magics sometimes failing due to indentation (#2630) + ## 21.11b1 ### _Black_ diff --git a/src/black/__init__.py b/src/black/__init__.py index f951b697361..8c940d3ac26 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -945,7 +945,8 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo def validate_cell(src: str) -> None: """Check that cell does not already contain TransformerManager transformations, - or cell magics, which might cause tokenizer_rt to break because of indentations. + or non-Python cell magics, which might cause tokenizer_rt to break because of + indentations. If a cell contains ``!ls``, then it'll be transformed to ``get_ipython().system('ls')``. However, if the cell originally contained From 00fb86b6927972485e8b53655cb1c127a0136939 Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 07:18:03 +0000 Subject: [PATCH 06/12] add note in documentation that non-Python cell magics are skipped too --- docs/faq.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/faq.md b/docs/faq.md index 72bae6b389d..88bf35b1e6a 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -47,6 +47,7 @@ _Black_ is timid about formatting Jupyter Notebooks. Cells containing any of the following will not be formatted: - automagics (e.g. `pip install black`) +- non-Python cell magics (e.g. `%%writeline`) - multiline magics, e.g.: ```python From 8994c9eff6b34d67c975103703b07a59fde4d3d7 Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 15:33:23 +0000 Subject: [PATCH 07/12] flip validate_cell cell magics for allowlist and remove later unnecessary check --- src/black/__init__.py | 6 ++++-- src/black/handle_ipynb_magics.py | 21 ++++++--------------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 8c940d3ac26..edb3cbf68c6 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -57,7 +57,7 @@ remove_trailing_semicolon, put_trailing_semicolon_back, TRANSFORMED_MAGICS, - NON_PYTHON_CELL_MAGICS, + PYTHON_CELL_MAGICS, jupyter_dependencies_are_installed, ) @@ -962,7 +962,9 @@ def validate_cell(src: str) -> None: """ if any(transformed_magic in src for transformed_magic in TRANSFORMED_MAGICS): raise NothingChanged - if any("%%" + cell_magic in src for cell_magic in NON_PYTHON_CELL_MAGICS): + if src[:2] == "%%" and src.split()[0] not in ( + "%%" + magic for magic in PYTHON_CELL_MAGICS + ): raise NothingChanged diff --git a/src/black/handle_ipynb_magics.py b/src/black/handle_ipynb_magics.py index 2fe6739209d..faea80f6fb6 100644 --- a/src/black/handle_ipynb_magics.py +++ b/src/black/handle_ipynb_magics.py @@ -37,20 +37,13 @@ "ESCAPED_NL", ) ) -NON_PYTHON_CELL_MAGICS = frozenset( +PYTHON_CELL_MAGICS = frozenset( ( - "bash", - "html", - "javascript", - "js", - "latex", - "markdown", - "perl", - "ruby", - "script", - "sh", - "svg", - "writefile", + "capture", + "prun", + "pypy", + "time", + "timeit", ) ) TOKEN_HEX = secrets.token_hex @@ -230,8 +223,6 @@ def replace_cell_magics(src: str) -> Tuple[str, List[Replacement]]: cell_magic_finder.visit(tree) if cell_magic_finder.cell_magic is None: return src, replacements - if cell_magic_finder.cell_magic.name in NON_PYTHON_CELL_MAGICS: - raise NothingChanged header = cell_magic_finder.cell_magic.header mask = get_token(src, header) replacements.append(Replacement(mask=mask, src=header)) From f12e58d24a5a5fe8ced1f9193f2afbfe28436c80 Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 15:58:48 +0000 Subject: [PATCH 08/12] replace %t with %timeit in test as we are now allowlisting python cell magics and %t would be a user-defined custom one --- tests/test_ipynb.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_ipynb.py b/tests/test_ipynb.py index de398ecbadf..ff005de45d2 100644 --- a/tests/test_ipynb.py +++ b/tests/test_ipynb.py @@ -129,9 +129,9 @@ def test_magic_noop() -> None: def test_cell_magic_with_magic() -> None: - src = "%%t -n1\nls =!ls" + src = "%%timeit -n1\nls =!ls" result = format_cell(src, fast=True, mode=JUPYTER_MODE) - expected = "%%t -n1\nls = !ls" + expected = "%%timeit -n1\nls = !ls" assert result == expected From 858a7c16469c3c7b967cf6b15d3b0436c4a71617 Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 16:15:35 +0000 Subject: [PATCH 09/12] added further python cell magics --- src/black/handle_ipynb_magics.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/black/handle_ipynb_magics.py b/src/black/handle_ipynb_magics.py index faea80f6fb6..cbf42f7364f 100644 --- a/src/black/handle_ipynb_magics.py +++ b/src/black/handle_ipynb_magics.py @@ -42,6 +42,9 @@ "capture", "prun", "pypy", + "python", + "python2", + "python3", "time", "timeit", ) From 1e8a0b1246e74406731981830a05203a35ad65bd Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 16:20:40 +0000 Subject: [PATCH 10/12] expand changelog --- CHANGES.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index ee4dcae410c..ad3d4d1641b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,7 +4,9 @@ ### _Black_ -- Fixed non-Python cell magics sometimes failing due to indentation (#2630) +- Cell magics are now only processed if they are known Python cell magics. Earlier, all + cell magics were tokenized, leading to possible indentation errors e.g. with + `%%writefile`. (#2630) ## 21.11b1 From 744820647cc1728d0c4e82e20fcfbeaeb0fd6cad Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Fri, 19 Nov 2021 10:50:05 -0600 Subject: [PATCH 11/12] removing `%%python2` as `Black` does not support Python2 anymore --- src/black/handle_ipynb_magics.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/black/handle_ipynb_magics.py b/src/black/handle_ipynb_magics.py index cbf42f7364f..52de8fba0d9 100644 --- a/src/black/handle_ipynb_magics.py +++ b/src/black/handle_ipynb_magics.py @@ -43,7 +43,6 @@ "prun", "pypy", "python", - "python2", "python3", "time", "timeit", From aadf7e260481439273e18a4350ce757da89e4a4a Mon Sep 17 00:00:00 2001 From: Daniel Sparing Date: Mon, 29 Nov 2021 17:27:01 -0500 Subject: [PATCH 12/12] Update src/black/__init__.py Co-authored-by: Marco Edward Gorelli --- src/black/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 484ae98ab0e..13a281e8622 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -962,9 +962,7 @@ def validate_cell(src: str) -> None: """ if any(transformed_magic in src for transformed_magic in TRANSFORMED_MAGICS): raise NothingChanged - if src[:2] == "%%" and src.split()[0] not in ( - "%%" + magic for magic in PYTHON_CELL_MAGICS - ): + if src[:2] == "%%" and src.split()[0][2:] not in PYTHON_CELL_MAGICS: raise NothingChanged