From 0f8bd820e5f2d1042546a5d1ebf153e47d1e1383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Robert?= Date: Tue, 11 Jul 2023 10:03:30 +0200 Subject: [PATCH 1/8] PERF: minimize cost of introspection to detect jupyter dependencies --- CHANGES.md | 2 ++ src/black/handle_ipynb_magics.py | 27 +++++++++------------------ 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c61ee698c5d..701705abd3f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -30,6 +30,8 @@ +- minimize cost of introspection to detect jupyter dependencies (#3782) + ### Output diff --git a/src/black/handle_ipynb_magics.py b/src/black/handle_ipynb_magics.py index 2a2d62220e2..4438cfe5015 100644 --- a/src/black/handle_ipynb_magics.py +++ b/src/black/handle_ipynb_magics.py @@ -6,6 +6,7 @@ import secrets import sys from functools import lru_cache +from importlib.util import find_spec from typing import Dict, List, Optional, Tuple if sys.version_info >= (3, 10): @@ -57,24 +58,14 @@ class Replacement: @lru_cache def jupyter_dependencies_are_installed(*, verbose: bool, quiet: bool) -> bool: - try: - # isort: off - # tokenize_rt is less commonly installed than IPython - # and IPython is expensive to import - import tokenize_rt # noqa:F401 - import IPython # noqa:F401 - - # isort: on - except ModuleNotFoundError: - if verbose or not quiet: - msg = ( - "Skipping .ipynb files as Jupyter dependencies are not installed.\n" - 'You can fix this by running ``pip install "black[jupyter]"``' - ) - out(msg) - return False - else: - return True + retv = find_spec("tokenize_rt") is not None and find_spec("IPython") is not None + if not retv and (verbose or not quiet): + msg = ( + "Skipping .ipynb files as Jupyter dependencies are not installed.\n" + 'You can fix this by running ``pip install "black[jupyter]"``' + ) + out(msg) + return retv def remove_trailing_semicolon(src: str) -> Tuple[str, bool]: From b4f09b80467acea30156ef0db45a25768e83dd67 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 15 Jul 2023 22:01:13 -0700 Subject: [PATCH 2/8] rename --- src/black/handle_ipynb_magics.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/black/handle_ipynb_magics.py b/src/black/handle_ipynb_magics.py index 4438cfe5015..76a395375ff 100644 --- a/src/black/handle_ipynb_magics.py +++ b/src/black/handle_ipynb_magics.py @@ -58,14 +58,14 @@ class Replacement: @lru_cache def jupyter_dependencies_are_installed(*, verbose: bool, quiet: bool) -> bool: - retv = find_spec("tokenize_rt") is not None and find_spec("IPython") is not None - if not retv and (verbose or not quiet): + installed = find_spec("tokenize_rt") is not None and find_spec("IPython") is not None + if not installed and (verbose or not quiet): msg = ( "Skipping .ipynb files as Jupyter dependencies are not installed.\n" 'You can fix this by running ``pip install "black[jupyter]"``' ) out(msg) - return retv + return installed def remove_trailing_semicolon(src: str) -> Tuple[str, bool]: From f74c3210a553bfd9224aae46ff1768ee7d67aa35 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 15 Jul 2023 22:03:14 -0700 Subject: [PATCH 3/8] redundant arg --- src/black/__init__.py | 2 +- src/black/files.py | 2 +- src/black/handle_ipynb_magics.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 301c18f7338..923a51867b5 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -668,7 +668,7 @@ def get_sources( p = Path(f"{STDIN_PLACEHOLDER}{str(p)}") if p.suffix == ".ipynb" and not jupyter_dependencies_are_installed( - verbose=verbose, quiet=quiet + warn=verbose or not quiet ): continue diff --git a/src/black/files.py b/src/black/files.py index ef6895ee3af..368e4170d47 100644 --- a/src/black/files.py +++ b/src/black/files.py @@ -384,7 +384,7 @@ def gen_python_files( elif child.is_file(): if child.suffix == ".ipynb" and not jupyter_dependencies_are_installed( - verbose=verbose, quiet=quiet + warn=verbose or not quiet ): continue include_match = include.search(normalized_path) if include else True diff --git a/src/black/handle_ipynb_magics.py b/src/black/handle_ipynb_magics.py index 76a395375ff..46da3f0817b 100644 --- a/src/black/handle_ipynb_magics.py +++ b/src/black/handle_ipynb_magics.py @@ -57,9 +57,9 @@ class Replacement: @lru_cache -def jupyter_dependencies_are_installed(*, verbose: bool, quiet: bool) -> bool: +def jupyter_dependencies_are_installed(*, warn: bool) -> bool: installed = find_spec("tokenize_rt") is not None and find_spec("IPython") is not None - if not installed and (verbose or not quiet): + if not installed and warn: msg = ( "Skipping .ipynb files as Jupyter dependencies are not installed.\n" 'You can fix this by running ``pip install "black[jupyter]"``' From 1322e297d137d5065384ce81a8c90fcdb8f13b59 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 15 Jul 2023 22:03:55 -0700 Subject: [PATCH 4/8] black --- src/black/handle_ipynb_magics.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/black/handle_ipynb_magics.py b/src/black/handle_ipynb_magics.py index 46da3f0817b..55ef2267df8 100644 --- a/src/black/handle_ipynb_magics.py +++ b/src/black/handle_ipynb_magics.py @@ -58,7 +58,9 @@ class Replacement: @lru_cache def jupyter_dependencies_are_installed(*, warn: bool) -> bool: - installed = find_spec("tokenize_rt") is not None and find_spec("IPython") is not None + installed = ( + find_spec("tokenize_rt") is not None and find_spec("IPython") is not None + ) if not installed and warn: msg = ( "Skipping .ipynb files as Jupyter dependencies are not installed.\n" From 65ed92b211895922ca754ee577cea75fedf03a48 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 15 Jul 2023 22:25:37 -0700 Subject: [PATCH 5/8] changelog --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 701705abd3f..d7fdf497307 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -30,7 +30,7 @@ -- minimize cost of introspection to detect jupyter dependencies (#3782) +- Avoid importing `IPython` if notebook cells do not contain magics (#3748) ### Output From d3c4ea16c39c144132ead3e9cee76be981975d5b Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 15 Jul 2023 22:26:46 -0700 Subject: [PATCH 6/8] fix changelog --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index d7fdf497307..709c767b329 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -30,7 +30,7 @@ -- Avoid importing `IPython` if notebook cells do not contain magics (#3748) +- Avoid importing `IPython` if notebook cells do not contain magics (#3782) ### Output From 3a9b9b596701ff08ae0d38787f48f502439608e3 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 15 Jul 2023 22:32:14 -0700 Subject: [PATCH 7/8] fix tests --- tests/test_ipynb.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_ipynb.py b/tests/test_ipynb.py index 91e7901125b..0cca7512c1a 100644 --- a/tests/test_ipynb.py +++ b/tests/test_ipynb.py @@ -441,7 +441,7 @@ def test_cache_isnt_written_if_no_jupyter_deps_single( tmp_nb = tmp_path / "notebook.ipynb" tmp_nb.write_bytes(nb.read_bytes()) monkeypatch.setattr( - "black.jupyter_dependencies_are_installed", lambda verbose, quiet: False + "black.jupyter_dependencies_are_installed", lambda warn: False ) result = runner.invoke( main, [str(tmp_path / "notebook.ipynb"), f"--config={EMPTY_CONFIG}"] @@ -449,7 +449,7 @@ def test_cache_isnt_written_if_no_jupyter_deps_single( assert "No Python files are present to be formatted. Nothing to do" in result.output jupyter_dependencies_are_installed.cache_clear() monkeypatch.setattr( - "black.jupyter_dependencies_are_installed", lambda verbose, quiet: True + "black.jupyter_dependencies_are_installed", lambda warn: True ) result = runner.invoke( main, [str(tmp_path / "notebook.ipynb"), f"--config={EMPTY_CONFIG}"] @@ -466,13 +466,13 @@ def test_cache_isnt_written_if_no_jupyter_deps_dir( tmp_nb = tmp_path / "notebook.ipynb" tmp_nb.write_bytes(nb.read_bytes()) monkeypatch.setattr( - "black.files.jupyter_dependencies_are_installed", lambda verbose, quiet: False + "black.files.jupyter_dependencies_are_installed", lambda warn: False ) result = runner.invoke(main, [str(tmp_path), f"--config={EMPTY_CONFIG}"]) assert "No Python files are present to be formatted. Nothing to do" in result.output jupyter_dependencies_are_installed.cache_clear() monkeypatch.setattr( - "black.files.jupyter_dependencies_are_installed", lambda verbose, quiet: True + "black.files.jupyter_dependencies_are_installed", lambda warn: True ) result = runner.invoke(main, [str(tmp_path), f"--config={EMPTY_CONFIG}"]) assert "reformatted" in result.output From c81797039a3ee108a0a3e87d45bafbfc4f6b1bb7 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sun, 16 Jul 2023 00:01:51 -0700 Subject: [PATCH 8/8] fix --- tests/test_ipynb.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_ipynb.py b/tests/test_ipynb.py index 0cca7512c1a..c72c89b32c7 100644 --- a/tests/test_ipynb.py +++ b/tests/test_ipynb.py @@ -440,17 +440,13 @@ def test_cache_isnt_written_if_no_jupyter_deps_single( nb = get_case_path("jupyter", "notebook_trailing_newline.ipynb") tmp_nb = tmp_path / "notebook.ipynb" tmp_nb.write_bytes(nb.read_bytes()) - monkeypatch.setattr( - "black.jupyter_dependencies_are_installed", lambda warn: False - ) + monkeypatch.setattr("black.jupyter_dependencies_are_installed", lambda warn: False) result = runner.invoke( main, [str(tmp_path / "notebook.ipynb"), f"--config={EMPTY_CONFIG}"] ) assert "No Python files are present to be formatted. Nothing to do" in result.output jupyter_dependencies_are_installed.cache_clear() - monkeypatch.setattr( - "black.jupyter_dependencies_are_installed", lambda warn: True - ) + monkeypatch.setattr("black.jupyter_dependencies_are_installed", lambda warn: True) result = runner.invoke( main, [str(tmp_path / "notebook.ipynb"), f"--config={EMPTY_CONFIG}"] )