From dd4fa3fa674adef0236e2660ef3731ca1fae997c Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Thu, 25 Feb 2021 13:19:47 -0600 Subject: [PATCH 1/9] Add nbqa-yapf --- .pre-commit-hooks.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index fec4f824..f88d2125 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -78,3 +78,12 @@ require_serial: true types: [jupyter] additional_dependencies: [pyupgrade] +- id: nbqa-yapf + name: nbqa-yapf + description: "Run 'yapf' on a Jupyter Notebook" + entry: nbqa yapf + language: python + language_version: python3 + require_serial: true + types: [jupyter] + additional_dependencies: [yapf] From 00847cc4ad1fffe1f8ea7bcbda7647bb2f8bc489 Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Thu, 25 Feb 2021 13:24:09 -0600 Subject: [PATCH 2/9] Modify in-place. --- .pre-commit-hooks.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index f88d2125..9da19a09 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -81,7 +81,7 @@ - id: nbqa-yapf name: nbqa-yapf description: "Run 'yapf' on a Jupyter Notebook" - entry: nbqa yapf + entry: nbqa yapf --in-place language: python language_version: python3 require_serial: true From 33732388558b3346ffd7684bb381d7527583fc0b Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Thu, 25 Feb 2021 15:31:21 -0600 Subject: [PATCH 3/9] Add yapf tests. --- requirements-dev.txt | 1 + tests/tools/test_yapf.py | 309 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 310 insertions(+) create mode 100644 tests/tools/test_yapf.py diff --git a/requirements-dev.txt b/requirements-dev.txt index 9f476c73..f8db20e1 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -9,3 +9,4 @@ pylint pytest pytest-randomly pyupgrade +yapf diff --git a/tests/tools/test_yapf.py b/tests/tools/test_yapf.py new file mode 100644 index 00000000..146248ce --- /dev/null +++ b/tests/tools/test_yapf.py @@ -0,0 +1,309 @@ +"""Check that :code:`yapf` works as intended.""" + +import difflib +import operator +import os +import subprocess +from pathlib import Path +from shutil import copyfile +from textwrap import dedent +from typing import TYPE_CHECKING, Callable + +import pytest + +from nbqa.__main__ import main + +if TYPE_CHECKING: + from _pytest.capture import CaptureFixture + from py._path.local import LocalPath + +SPARKLES = "\N{sparkles}" +SHORTCAKE = "\N{shortcake}" +COLLISION = "\N{collision symbol}" +BROKEN_HEART = "\N{broken heart}" + + +def test_yapf_works(tmp_notebook_for_testing: Path, capsys: "CaptureFixture") -> None: + """ + Check yapf works. Should only reformat code cells. + + Parameters + ---------- + tmp_notebook_for_testing + Temporary copy of :code:`notebook_for_testing.ipynb`. + capsys + Pytest fixture to capture stdout and stderr. + """ + # check diff + with open(tmp_notebook_for_testing) as handle: + before = handle.readlines() + path = os.path.abspath(os.path.join("tests", "data", "notebook_for_testing.ipynb")) + + Path("setup.cfg").write_text( + dedent( + """\ + [nbqa.mutate] + yapf=1 + """ + ) + ) + with pytest.raises(SystemExit): + main(["yapf", "--in-place", path]) + Path("setup.cfg").unlink() + with open(tmp_notebook_for_testing) as handle: + after = handle.readlines() + + diff = difflib.unified_diff(before, after) + result = "".join([i for i in diff if any([i.startswith("+ "), i.startswith("- ")])]) + expected = dedent( + """\ + - "hello(3) " + + "hello(3)" + - " %time randint(5,10)" + + " %time randint(5, 10)" + - " %time pretty_print_object = pprint.PrettyPrinter(\\\\\\n", + - " indent=4, width=80, stream=sys.stdout, compact=True, depth=5\\\\\\n", + - " )\\n", + + " %time pretty_print_object = pprint.PrettyPrinter(indent=4,\\\\\\n", + + " width=80,\\\\\\n", + + " stream=sys.stdout,\\\\\\n", + + " compact=True,\\\\\\n", + + " depth=5)\\n", + """ + ) + assert result == expected + + # check out and err + out, err = capsys.readouterr() + expected_out = "" + expected_err = "" + assert out == expected_out + assert err == expected_err + + +def test_yapf_works_with_trailing_semicolons( + tmp_notebook_with_trailing_semicolon: Path, capsys: "CaptureFixture" +) -> None: + """ + Check yapf works. Should only reformat code cells. + + Parameters + ---------- + tmp_notebook_with_trailing_semicolon + Temporary copy of :code:`notebook_with_trailing_semicolon.ipynb`. + capsys + Pytest fixture to capture stdout and stderr. + """ + # check diff + with open(tmp_notebook_with_trailing_semicolon) as handle: + before = handle.readlines() + path = os.path.abspath( + os.path.join("tests", "data", "notebook_with_trailing_semicolon.ipynb") + ) + + Path("setup.cfg").write_text( + dedent( + """\ + [nbqa.mutate] + yapf=1 + """ + ) + ) + with pytest.raises(SystemExit): + main(["yapf", "--in-place", path]) + Path("setup.cfg").unlink() + with open(tmp_notebook_with_trailing_semicolon) as handle: + after = handle.readlines() + + diff = difflib.unified_diff(before, after) + result = "".join([i for i in diff if any([i.startswith("+ "), i.startswith("- ")])]) + expected = dedent( + """\ + - "import glob;\\n", + + "import glob\\n", + - " pass;\\n", + - " " + + " pass;" + """ + ) + assert result == expected + + # check out and err + out, err = capsys.readouterr() + expected_out = "" + expected_err = "" + assert out == expected_out + assert err == expected_err + + +def test_yapf_works_with_multiline( + tmp_notebook_with_multiline: Path, capsys: "CaptureFixture" +) -> None: + """ + Check yapf works. Should only reformat code cells. + + Parameters + ---------- + tmp_notebook_with_multiline + Temporary copy of :code:`clean_notebook_with_multiline.ipynb`. + capsys + Pytest fixture to capture stdout and stderr. + """ + # check diff + with open(tmp_notebook_with_multiline) as handle: + before = handle.readlines() + path = os.path.abspath( + os.path.join("tests", "data", "clean_notebook_with_multiline.ipynb") + ) + + Path("setup.cfg").write_text( + dedent( + """\ + [nbqa.mutate] + yapf=1 + """ + ) + ) + with pytest.raises(SystemExit): + main(["yapf", "--in-place", path]) + Path("setup.cfg").unlink() + with open(tmp_notebook_with_multiline) as handle: + after = handle.readlines() + + diff = difflib.unified_diff(before, after) + result = "".join([i for i in diff if any([i.startswith("+ "), i.startswith("- ")])]) + expected = dedent( + """\ + - "assert 1 + 1 == 2; assert 1 + 1 == 2;" + + "assert 1 + 1 == 2\\n", + + "assert 1 + 1 == 2;" + """ + ) + assert result == expected + + # check out and err + out, err = capsys.readouterr() + expected_out = "" + expected_err = "" + assert out == expected_out + assert err == expected_err + + +def test_yapf_multiple_files(tmp_test_data: Path) -> None: + """ + Check yapf works when running on a directory. Should reformat notebooks. + + Parameters + ---------- + tmp_test_data + Temporary copy of test data. + """ + # check diff + with open(str(tmp_test_data / "notebook_for_testing.ipynb")) as handle: + before = handle.readlines() + path = os.path.abspath(os.path.join("tests", "data")) + + Path("setup.cfg").write_text( + dedent( + """\ + [nbqa.mutate] + yapf=1 + """ + ) + ) + with pytest.raises(SystemExit): + main(["yapf", "--in-place", "--recursive", path]) + Path("setup.cfg").unlink() + with open(str(tmp_test_data / "notebook_for_testing.ipynb")) as handle: + after = handle.readlines() + + diff = difflib.unified_diff(before, after) + assert "".join(diff) != "" + + +def test_successive_runs_using_yapf(tmpdir: "LocalPath") -> None: + """Check yapf returns 0 on the second run given a dirty notebook.""" + src_notebook = Path(os.path.join("tests", "data", "notebook_for_testing.ipynb")) + test_notebook = Path(tmpdir) / src_notebook.name + copyfile(src_notebook, test_notebook) + + def run_yapf( + test_notebook: str, content_compare_op: Callable[[float, float], bool] + ) -> bool: + """Run yapf using nbqa and validate the output.""" + with open(test_notebook, "r") as f: + before_contents = f.readlines() + output = subprocess.run(["nbqa", "yapf", "--in-place", test_notebook, "--nbqa-mutate"]) + with open(test_notebook, "r") as f: + after_contents = f.readlines() + return output.returncode == 0 and content_compare_op( + before_contents, after_contents + ) + + assert run_yapf(str(test_notebook), operator.ne) + assert run_yapf(str(test_notebook), operator.eq) + + +def test_yapf_works_with_commented_magics(capsys: "CaptureFixture") -> None: + """ + Check yapf works with notebooks with commented-out magics. + + Parameters + ---------- + capsys + Pytest fixture to capture stdout and stderr. + """ + path = os.path.abspath(os.path.join("tests", "data", "commented_out_magic.ipynb")) + + with pytest.raises(SystemExit): + main(["yapf", "--in-place", path, "--nbqa-diff"]) + + out, err = capsys.readouterr() + expected_out = f"""\ +\x1b[1mCell 1\x1b[0m +------ +--- {path} ++++ {path} +@@ -1,2 +1 @@ +\x1b[31m-[1, 2, +\x1b[0m\x1b[31m-3, 4] +\x1b[0m\x1b[32m+[1, 2, 3, 4] +\x1b[0m +To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff` +""" + expected_err = "" + assert expected_out == out + assert expected_err == err + + +def test_yapf_works_with_leading_comment(capsys: "CaptureFixture") -> None: + """ + Check yapf works with notebooks with commented-out magics. + + Parameters + ---------- + capsys + Pytest fixture to capture stdout and stderr. + """ + path = os.path.abspath(os.path.join("tests", "data", "starting_with_comment.ipynb")) + + with pytest.raises(SystemExit): + main(["yapf", "--in-place", path, "--nbqa-diff"]) + + out, err = capsys.readouterr() + expected_out = f"""\ +\x1b[1mCell 3\x1b[0m +------ +--- {path} ++++ {path} +@@ -1,3 +1,3 @@ + # export +\x1b[31m-def example_func(hi = "yo"): +\x1b[0m\x1b[32m+def example_func(hi="yo"): +\x1b[0m pass + +To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff` +""" + expected_err = "" + assert expected_out == out + assert expected_err == err From 263d8a10609b2d088f83841df1e4d5c2045054b6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 25 Feb 2021 21:32:54 +0000 Subject: [PATCH 4/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/tools/test_yapf.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/tools/test_yapf.py b/tests/tools/test_yapf.py index 146248ce..456f42be 100644 --- a/tests/tools/test_yapf.py +++ b/tests/tools/test_yapf.py @@ -231,10 +231,12 @@ def run_yapf( test_notebook: str, content_compare_op: Callable[[float, float], bool] ) -> bool: """Run yapf using nbqa and validate the output.""" - with open(test_notebook, "r") as f: + with open(test_notebook) as f: before_contents = f.readlines() - output = subprocess.run(["nbqa", "yapf", "--in-place", test_notebook, "--nbqa-mutate"]) - with open(test_notebook, "r") as f: + output = subprocess.run( + ["nbqa", "yapf", "--in-place", test_notebook, "--nbqa-mutate"] + ) + with open(test_notebook) as f: after_contents = f.readlines() return output.returncode == 0 and content_compare_op( before_contents, after_contents From 6a495746631d4086d7b7bd47adf32e3657cd1ff0 Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Thu, 25 Feb 2021 15:35:28 -0600 Subject: [PATCH 5/9] Add yapf example to docs. --- docs/examples.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/examples.rst b/docs/examples.rst index a5869112..7a04affc 100644 --- a/docs/examples.rst +++ b/docs/examples.rst @@ -75,6 +75,12 @@ Upgrade your syntax with `pyupgrade`_: $ nbqa pyupgrade my_notebook.ipynb --py36-plus --nbqa-mutate Rewriting my_notebook.ipynb +Format code with `yapf`_: + +.. code:: console + + $ nbqa yapf --in-place my_notebook.ipynb --nbqa-mutate + .. _autoflake: https://github.com/myint/autoflake .. _black: https://black.readthedocs.io/en/stable/ .. _check-ast: https://github.com/pre-commit/pre-commit-hooks#check-ast @@ -84,3 +90,4 @@ Upgrade your syntax with `pyupgrade`_: .. _mypy: http://mypy-lang.org/ .. _pylint: https://www.pylint.org/ .. _pyupgrade: https://github.com/asottile/pyupgrade +.. _yapf: https://github.com/google/yapf From f2962765a776d9a8eddca2a3dd8ca5e9503bdb54 Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Thu, 25 Feb 2021 15:41:24 -0600 Subject: [PATCH 6/9] Fix type hints. --- tests/tools/test_yapf.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/tools/test_yapf.py b/tests/tools/test_yapf.py index 456f42be..4a98dee2 100644 --- a/tests/tools/test_yapf.py +++ b/tests/tools/test_yapf.py @@ -7,7 +7,7 @@ from pathlib import Path from shutil import copyfile from textwrap import dedent -from typing import TYPE_CHECKING, Callable +from typing import TYPE_CHECKING, Callable, List import pytest @@ -228,16 +228,16 @@ def test_successive_runs_using_yapf(tmpdir: "LocalPath") -> None: copyfile(src_notebook, test_notebook) def run_yapf( - test_notebook: str, content_compare_op: Callable[[float, float], bool] + test_notebook: str, content_compare_op: Callable[[List[str], List[str]], bool] ) -> bool: """Run yapf using nbqa and validate the output.""" - with open(test_notebook) as f: - before_contents = f.readlines() + with open(test_notebook) as test_file: + before_contents: List[str] = test_file.readlines() output = subprocess.run( ["nbqa", "yapf", "--in-place", test_notebook, "--nbqa-mutate"] ) - with open(test_notebook) as f: - after_contents = f.readlines() + with open(test_notebook) as test_file: + after_contents: List[str] = test_file.readlines() return output.returncode == 0 and content_compare_op( before_contents, after_contents ) From dd73d5764d99fb0a7128378eafcd7339e62657b5 Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Thu, 25 Feb 2021 15:46:26 -0600 Subject: [PATCH 7/9] Test cleanup. --- tests/tools/test_yapf.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/tools/test_yapf.py b/tests/tools/test_yapf.py index 4a98dee2..433b35d8 100644 --- a/tests/tools/test_yapf.py +++ b/tests/tools/test_yapf.py @@ -17,11 +17,6 @@ from _pytest.capture import CaptureFixture from py._path.local import LocalPath -SPARKLES = "\N{sparkles}" -SHORTCAKE = "\N{shortcake}" -COLLISION = "\N{collision symbol}" -BROKEN_HEART = "\N{broken heart}" - def test_yapf_works(tmp_notebook_for_testing: Path, capsys: "CaptureFixture") -> None: """ @@ -231,13 +226,13 @@ def run_yapf( test_notebook: str, content_compare_op: Callable[[List[str], List[str]], bool] ) -> bool: """Run yapf using nbqa and validate the output.""" - with open(test_notebook) as test_file: - before_contents: List[str] = test_file.readlines() + with open(test_notebook) as handle: + before_contents: List[str] = handle.readlines() output = subprocess.run( ["nbqa", "yapf", "--in-place", test_notebook, "--nbqa-mutate"] ) - with open(test_notebook) as test_file: - after_contents: List[str] = test_file.readlines() + with open(test_notebook) as handle: + after_contents: List[str] = handle.readlines() return output.returncode == 0 and content_compare_op( before_contents, after_contents ) From 8379462dae105111e005ce3cd3b2865f5b58996c Mon Sep 17 00:00:00 2001 From: Marco Gorelli Date: Sat, 17 Apr 2021 18:14:24 +0100 Subject: [PATCH 8/9] check for fase positives --- nbqa/__main__.py | 54 ++++--- nbqa/output_parser.py | 13 +- nbqa/replace_source.py | 34 +++- tests/tools/test_yapf.py | 338 +++++++-------------------------------- 4 files changed, 125 insertions(+), 314 deletions(-) diff --git a/nbqa/__main__.py b/nbqa/__main__.py index c93f43fc..f15ced8f 100644 --- a/nbqa/__main__.py +++ b/nbqa/__main__.py @@ -18,7 +18,7 @@ from nbqa.find_root import find_project_root from nbqa.notebook_info import NotebookInfo from nbqa.optional import metadata -from nbqa.output_parser import map_python_line_to_nb_lines +from nbqa.output_parser import Output, map_python_line_to_nb_lines from nbqa.text import BOLD, RESET BASE_ERROR_MESSAGE = dedent( @@ -180,7 +180,7 @@ def _replace_temp_python_file_references_in_out_err( notebook: Path, out: str, err: str, -) -> Tuple[str, str]: +) -> Output: """ Replace references to temporary Python file with references to notebook. @@ -199,10 +199,8 @@ def _replace_temp_python_file_references_in_out_err( Returns ------- - out - Stdout with temporary directory replaced by current working directory. - err - Stderr with temporary directory replaced by current working directory. + Output + Stdout, stderr with temporary directory replaced by current working directory. """ # 1. Relative path is used because some tools like pylint always report only # the relative path of the file(relative to project root), @@ -231,7 +229,7 @@ def _replace_temp_python_file_references_in_out_err( out = out.replace(temp_python_file.stem, notebook.stem) err = err.replace(temp_python_file.stem, notebook.stem) - return out, err + return Output(out, err) def _create_blank_init_files( @@ -382,7 +380,7 @@ def _run_command( tmpdirname: str, cmd_args: Sequence[str], args: Sequence[Path], -) -> Tuple[str, str, int, bool]: +) -> Tuple[Output, int, bool]: """ Run third-party tool against given file or directory. @@ -399,10 +397,8 @@ def _run_command( Returns ------- - out - Captured stdout from running third-party tool. - err - Captured stderr from running third-party tool. + output + Captured stdout, stderr from running third-party tool. output_code Return code from third-party tool. mutated @@ -440,7 +436,7 @@ def _run_command( out = output.stdout err = output.stderr - return out, err, output_code, mutated + return Output(out, err), output_code, mutated def _get_command_not_found_msg(command: str) -> str: @@ -569,7 +565,7 @@ def _run_on_one_root_dir( _create_blank_init_files(notebook, tmpdirname, project_root) - out, err, output_code, mutated = _run_command( + output, output_code, mutated = _run_command( cli_args.command, tmpdirname, configs.nbqa_addopts, @@ -578,15 +574,16 @@ def _run_on_one_root_dir( ), ) + actually_mutated = False for notebook, temp_python_file in nb_to_py_mapping.items(): - out, err = _replace_temp_python_file_references_in_out_err( - tmpdirname, temp_python_file, notebook, out, err + output = _replace_temp_python_file_references_in_out_err( + tmpdirname, temp_python_file, notebook, output.out, output.err ) try: - out, err = map_python_line_to_nb_lines( + output = map_python_line_to_nb_lines( cli_args.command, - out, - err, + output.out, + output.err, notebook, nb_info_mapping[notebook].cell_mappings, ) @@ -615,10 +612,13 @@ def _run_on_one_root_dir( raise SystemExit(msg) try: - REPLACE_FUNCTION[configs.nbqa_diff]( - temp_python_file, - notebook, - nb_info_mapping[notebook], + actually_mutated = ( + REPLACE_FUNCTION[configs.nbqa_diff]( + temp_python_file, + notebook, + nb_info_mapping[notebook], + ) + or actually_mutated ) except Exception as exc: raise RuntimeError( @@ -627,8 +627,12 @@ def _run_on_one_root_dir( ) ) from exc - sys.stdout.write(out) - sys.stderr.write(err) + sys.stdout.write(output.out) + sys.stderr.write(output.err) + + if mutated and not actually_mutated: + output_code = 0 + mutated = False if configs.nbqa_diff: if mutated: diff --git a/nbqa/output_parser.py b/nbqa/output_parser.py index b68f8ac7..b928f99f 100644 --- a/nbqa/output_parser.py +++ b/nbqa/output_parser.py @@ -3,7 +3,7 @@ import re from functools import partial from pathlib import Path -from typing import Callable, Mapping, Match, Sequence, Tuple, Union +from typing import Callable, Mapping, Match, NamedTuple, Sequence, Tuple, Union def _line_to_cell(match: Match[str], cell_mapping: Mapping[int, str]) -> str: @@ -11,6 +11,13 @@ def _line_to_cell(match: Match[str], cell_mapping: Mapping[int, str]) -> str: return str(cell_mapping[int(match.group())]) +class Output(NamedTuple): + """Captured stdout and stderr.""" + + out: str + err: str + + def _get_pattern( notebook: Path, command: str, cell_mapping: Mapping[int, str] ) -> Sequence[Tuple[str, Union[str, Callable[[Match[str]], str]]]]: @@ -58,7 +65,7 @@ def _get_pattern( def map_python_line_to_nb_lines( command: str, out: str, err: str, notebook: Path, cell_mapping: Mapping[int, str] -) -> Tuple[str, str]: +) -> Output: """ Make sure stdout and stderr make reference to Jupyter Notebook cells and lines. @@ -89,4 +96,4 @@ def map_python_line_to_nb_lines( out = re.sub(pattern_, substitution_, out, flags=re.MULTILINE) err = re.sub(pattern_, substitution_, err, flags=re.MULTILINE) - return out, err + return Output(out, err) diff --git a/nbqa/replace_source.py b/nbqa/replace_source.py index 28d3da83..be8e4c6b 100644 --- a/nbqa/replace_source.py +++ b/nbqa/replace_source.py @@ -4,6 +4,7 @@ The converted file will have had the third-party tool run against it by now. """ +import copy import json import sys from difflib import unified_diff @@ -156,7 +157,7 @@ def _notebook_code_cells( yield cell -def mutate(python_file: "Path", notebook: "Path", notebook_info: NotebookInfo) -> None: +def mutate(python_file: "Path", notebook: "Path", notebook_info: NotebookInfo) -> bool: """ Replace :code:`source` code cells of original notebook. @@ -168,8 +169,14 @@ def mutate(python_file: "Path", notebook: "Path", notebook_info: NotebookInfo) - Jupyter Notebook third-party tool is run against (unmodified). notebook_info Information about notebook cells used for processing + + Returns + ------- + bool + Whether mutation actually happened. """ notebook_json = json.loads(notebook.read_text(encoding="utf-8")) + original_notebook_json = copy.deepcopy(notebook_json) pycells = _get_pycells(python_file) for code_cell_number, cell in enumerate( @@ -179,14 +186,18 @@ def mutate(python_file: "Path", notebook: "Path", notebook_info: NotebookInfo) - continue cell["source"] = _get_new_source(code_cell_number, notebook_info, next(pycells)) + if original_notebook_json == notebook_json: + return False + temp_notebook = python_file.parent / notebook.name temp_notebook.write_text( f"{json.dumps(notebook_json, indent=1, ensure_ascii=False)}\n", encoding="utf-8" ) move(str(temp_notebook), str(notebook)) + return True -def _print_diff(code_cell_number: int, cell_diff: Iterator[str]) -> None: +def _print_diff(code_cell_number: int, cell_diff: Iterator[str]) -> bool: """ Print diff between cells, colouring as appropriate. @@ -196,6 +207,11 @@ def _print_diff(code_cell_number: int, cell_diff: Iterator[str]) -> None: Current cell number cell_diff Diff between original and new versions of cell. + + Returns + ------- + bool + Whether non-null diff was printed. """ line_changes = [] for line in cell_diff: @@ -212,9 +228,11 @@ def _print_diff(code_cell_number: int, cell_diff: Iterator[str]) -> None: header = f"Cell {code_cell_number}" headers = [f"{BOLD}{header}{RESET}\n", f"{'-'*len(header)}\n"] sys.stdout.writelines(headers + line_changes + ["\n"]) + return True + return False -def diff(python_file: "Path", notebook: "Path", notebook_info: NotebookInfo) -> None: +def diff(python_file: "Path", notebook: "Path", notebook_info: NotebookInfo) -> bool: """ View diff between new source of code cells and original sources. @@ -226,11 +244,18 @@ def diff(python_file: "Path", notebook: "Path", notebook_info: NotebookInfo) -> Jupyter Notebook third-party tool is run against (unmodified). notebook_info Information about notebook cells used for processing + + Returns + ------- + bool + Whether non-null diff was produced. """ notebook_json = json.loads(notebook.read_text(encoding="utf-8")) pycells = _get_pycells(python_file) + actually_mutated = False + for code_cell_number, cell in enumerate( _notebook_code_cells(notebook_json), start=1 ): @@ -245,4 +270,5 @@ def diff(python_file: "Path", notebook: "Path", notebook_info: NotebookInfo) -> fromfile=str(notebook), tofile=str(notebook), ) - _print_diff(code_cell_number, cell_diff) + actually_mutated = _print_diff(code_cell_number, cell_diff) or actually_mutated + return actually_mutated diff --git a/tests/tools/test_yapf.py b/tests/tools/test_yapf.py index 433b35d8..a1b971bf 100644 --- a/tests/tools/test_yapf.py +++ b/tests/tools/test_yapf.py @@ -1,13 +1,9 @@ """Check that :code:`yapf` works as intended.""" -import difflib -import operator import os -import subprocess from pathlib import Path from shutil import copyfile -from textwrap import dedent -from typing import TYPE_CHECKING, Callable, List +from typing import TYPE_CHECKING import pytest @@ -18,289 +14,67 @@ from py._path.local import LocalPath -def test_yapf_works(tmp_notebook_for_testing: Path, capsys: "CaptureFixture") -> None: - """ - Check yapf works. Should only reformat code cells. - - Parameters - ---------- - tmp_notebook_for_testing - Temporary copy of :code:`notebook_for_testing.ipynb`. - capsys - Pytest fixture to capture stdout and stderr. - """ - # check diff - with open(tmp_notebook_for_testing) as handle: - before = handle.readlines() - path = os.path.abspath(os.path.join("tests", "data", "notebook_for_testing.ipynb")) - - Path("setup.cfg").write_text( - dedent( - """\ - [nbqa.mutate] - yapf=1 - """ - ) - ) - with pytest.raises(SystemExit): - main(["yapf", "--in-place", path]) - Path("setup.cfg").unlink() - with open(tmp_notebook_for_testing) as handle: - after = handle.readlines() - - diff = difflib.unified_diff(before, after) - result = "".join([i for i in diff if any([i.startswith("+ "), i.startswith("- ")])]) - expected = dedent( - """\ - - "hello(3) " - + "hello(3)" - - " %time randint(5,10)" - + " %time randint(5, 10)" - - " %time pretty_print_object = pprint.PrettyPrinter(\\\\\\n", - - " indent=4, width=80, stream=sys.stdout, compact=True, depth=5\\\\\\n", - - " )\\n", - + " %time pretty_print_object = pprint.PrettyPrinter(indent=4,\\\\\\n", - + " width=80,\\\\\\n", - + " stream=sys.stdout,\\\\\\n", - + " compact=True,\\\\\\n", - + " depth=5)\\n", - """ - ) - assert result == expected - - # check out and err - out, err = capsys.readouterr() - expected_out = "" - expected_err = "" - assert out == expected_out - assert err == expected_err - - -def test_yapf_works_with_trailing_semicolons( - tmp_notebook_with_trailing_semicolon: Path, capsys: "CaptureFixture" -) -> None: - """ - Check yapf works. Should only reformat code cells. - - Parameters - ---------- - tmp_notebook_with_trailing_semicolon - Temporary copy of :code:`notebook_with_trailing_semicolon.ipynb`. - capsys - Pytest fixture to capture stdout and stderr. - """ - # check diff - with open(tmp_notebook_with_trailing_semicolon) as handle: - before = handle.readlines() - path = os.path.abspath( - os.path.join("tests", "data", "notebook_with_trailing_semicolon.ipynb") - ) - - Path("setup.cfg").write_text( - dedent( - """\ - [nbqa.mutate] - yapf=1 - """ - ) - ) - with pytest.raises(SystemExit): - main(["yapf", "--in-place", path]) - Path("setup.cfg").unlink() - with open(tmp_notebook_with_trailing_semicolon) as handle: - after = handle.readlines() - - diff = difflib.unified_diff(before, after) - result = "".join([i for i in diff if any([i.startswith("+ "), i.startswith("- ")])]) - expected = dedent( - """\ - - "import glob;\\n", - + "import glob\\n", - - " pass;\\n", - - " " - + " pass;" - """ - ) - assert result == expected - - # check out and err - out, err = capsys.readouterr() - expected_out = "" - expected_err = "" - assert out == expected_out - assert err == expected_err - - -def test_yapf_works_with_multiline( - tmp_notebook_with_multiline: Path, capsys: "CaptureFixture" +def test_successive_runs_using_yapf( + tmpdir: "LocalPath", capsys: "CaptureFixture" ) -> None: - """ - Check yapf works. Should only reformat code cells. - - Parameters - ---------- - tmp_notebook_with_multiline - Temporary copy of :code:`clean_notebook_with_multiline.ipynb`. - capsys - Pytest fixture to capture stdout and stderr. - """ - # check diff - with open(tmp_notebook_with_multiline) as handle: - before = handle.readlines() - path = os.path.abspath( - os.path.join("tests", "data", "clean_notebook_with_multiline.ipynb") - ) - - Path("setup.cfg").write_text( - dedent( - """\ - [nbqa.mutate] - yapf=1 - """ - ) - ) - with pytest.raises(SystemExit): - main(["yapf", "--in-place", path]) - Path("setup.cfg").unlink() - with open(tmp_notebook_with_multiline) as handle: - after = handle.readlines() - - diff = difflib.unified_diff(before, after) - result = "".join([i for i in diff if any([i.startswith("+ "), i.startswith("- ")])]) - expected = dedent( - """\ - - "assert 1 + 1 == 2; assert 1 + 1 == 2;" - + "assert 1 + 1 == 2\\n", - + "assert 1 + 1 == 2;" - """ - ) - assert result == expected - - # check out and err - out, err = capsys.readouterr() - expected_out = "" - expected_err = "" - assert out == expected_out - assert err == expected_err - - -def test_yapf_multiple_files(tmp_test_data: Path) -> None: - """ - Check yapf works when running on a directory. Should reformat notebooks. - - Parameters - ---------- - tmp_test_data - Temporary copy of test data. - """ - # check diff - with open(str(tmp_test_data / "notebook_for_testing.ipynb")) as handle: - before = handle.readlines() - path = os.path.abspath(os.path.join("tests", "data")) - - Path("setup.cfg").write_text( - dedent( - """\ - [nbqa.mutate] - yapf=1 - """ - ) - ) - with pytest.raises(SystemExit): - main(["yapf", "--in-place", "--recursive", path]) - Path("setup.cfg").unlink() - with open(str(tmp_test_data / "notebook_for_testing.ipynb")) as handle: - after = handle.readlines() - - diff = difflib.unified_diff(before, after) - assert "".join(diff) != "" - - -def test_successive_runs_using_yapf(tmpdir: "LocalPath") -> None: """Check yapf returns 0 on the second run given a dirty notebook.""" src_notebook = Path(os.path.join("tests", "data", "notebook_for_testing.ipynb")) test_notebook = Path(tmpdir) / src_notebook.name copyfile(src_notebook, test_notebook) - - def run_yapf( - test_notebook: str, content_compare_op: Callable[[List[str], List[str]], bool] - ) -> bool: - """Run yapf using nbqa and validate the output.""" - with open(test_notebook) as handle: - before_contents: List[str] = handle.readlines() - output = subprocess.run( - ["nbqa", "yapf", "--in-place", test_notebook, "--nbqa-mutate"] - ) - with open(test_notebook) as handle: - after_contents: List[str] = handle.readlines() - return output.returncode == 0 and content_compare_op( - before_contents, after_contents - ) - - assert run_yapf(str(test_notebook), operator.ne) - assert run_yapf(str(test_notebook), operator.eq) - - -def test_yapf_works_with_commented_magics(capsys: "CaptureFixture") -> None: - """ - Check yapf works with notebooks with commented-out magics. - - Parameters - ---------- - capsys - Pytest fixture to capture stdout and stderr. - """ - path = os.path.abspath(os.path.join("tests", "data", "commented_out_magic.ipynb")) - with pytest.raises(SystemExit): - main(["yapf", "--in-place", path, "--nbqa-diff"]) - - out, err = capsys.readouterr() - expected_out = f"""\ -\x1b[1mCell 1\x1b[0m ------- ---- {path} -+++ {path} -@@ -1,2 +1 @@ -\x1b[31m-[1, 2, -\x1b[0m\x1b[31m-3, 4] -\x1b[0m\x1b[32m+[1, 2, 3, 4] -\x1b[0m -To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff` -""" - expected_err = "" - assert expected_out == out - assert expected_err == err - - -def test_yapf_works_with_leading_comment(capsys: "CaptureFixture") -> None: - """ - Check yapf works with notebooks with commented-out magics. - - Parameters - ---------- - capsys - Pytest fixture to capture stdout and stderr. - """ - path = os.path.abspath(os.path.join("tests", "data", "starting_with_comment.ipynb")) + main(["yapf", str(test_notebook), "--in-place", "--nbqa-diff"]) + out, _ = capsys.readouterr() + expected_out = ( + "\x1b[1mCell 2\x1b[0m\n" + "------\n" + f"--- {str(test_notebook)}\n" + f"+++ {str(test_notebook)}\n" + "@@ -16,4 +16,4 @@\n" + " \n" + " \n" + " !ls\n" + "\x1b[31m-hello(3) \n" + "\x1b[0m\x1b[32m+hello(3)\n" + "\x1b[0m\n" + "\x1b[1mCell 4\x1b[0m\n" + "------\n" + f"--- {str(test_notebook)}\n" + f"+++ {str(test_notebook)}\n" + "@@ -1,4 +1,4 @@\n" + " from random import randint\n" + " \n" + " if __debug__:\n" + "\x1b[31m- %time randint(5,10)\n" + "\x1b[0m\x1b[32m+ %time randint(5, 10)\n" + "\x1b[0m\n" + "\x1b[1mCell 5\x1b[0m\n" + "------\n" + f"--- {str(test_notebook)}\n" + f"+++ {str(test_notebook)}\n" + "@@ -2,8 +2,10 @@\n" + " import sys\n" + " \n" + " if __debug__:\n" + "\x1b[31m- %time pretty_print_object = pprint.PrettyPrinter(\\\n" + "\x1b[0m\x1b[31m- indent=4, width=80, stream=sys.stdout, compact=True, depth=5\\\n" # pylint: disable=C0301 # noqa: E501 + "\x1b[0m\x1b[31m- )\n" + "\x1b[0m\x1b[32m+ %time pretty_print_object = pprint.PrettyPrinter(indent=4,\\\n" + "\x1b[0m\x1b[32m+ width=80,\\\n" + "\x1b[0m\x1b[32m+ stream=sys.stdout,\\\n" # pylint: disable=C0301 # noqa: E501 + "\x1b[0m\x1b[32m+ compact=True,\\\n" + "\x1b[0m\x1b[32m+ depth=5)\n" + "\x1b[0m \n" + ' pretty_print_object.isreadable(["Hello", "World"])\n' + "\n" + "To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff`\n" + ) + assert out == expected_out with pytest.raises(SystemExit): - main(["yapf", "--in-place", path, "--nbqa-diff"]) - - out, err = capsys.readouterr() - expected_out = f"""\ -\x1b[1mCell 3\x1b[0m ------- ---- {path} -+++ {path} -@@ -1,3 +1,3 @@ - # export -\x1b[31m-def example_func(hi = "yo"): -\x1b[0m\x1b[32m+def example_func(hi="yo"): -\x1b[0m pass + main(["yapf", str(test_notebook), "--in-place", "--nbqa-mutate"]) + with pytest.raises(SystemExit): + main(["yapf", str(test_notebook), "--in-place", "--nbqa-diff"]) -To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff` -""" - expected_err = "" - assert expected_out == out - assert expected_err == err + out, _ = capsys.readouterr() + expected_out = "" + assert out == expected_out From bd77929ece1b0d4dbdb063b07d33bd019697db57 Mon Sep 17 00:00:00 2001 From: Marco Gorelli Date: Sat, 17 Apr 2021 18:15:17 +0100 Subject: [PATCH 9/9] doc --- nbqa/output_parser.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/nbqa/output_parser.py b/nbqa/output_parser.py index b928f99f..55387988 100644 --- a/nbqa/output_parser.py +++ b/nbqa/output_parser.py @@ -84,11 +84,8 @@ def map_python_line_to_nb_lines( Returns ------- - str - Stdout with references to temporary Python file's lines replaced with references - to notebook's cells and lines. - str - Stderr with references to temporary Python file's lines replaced with references + Output + Stdout, stderr with references to temporary Python file's lines replaced with references to notebook's cells and lines. """ patterns = _get_pattern(notebook, command, cell_mapping)