From 9ad9a6fa1615d06612a2bec838b846d41f6601fc Mon Sep 17 00:00:00 2001 From: Murilo Cunha Date: Fri, 11 Nov 2022 17:26:10 +0100 Subject: [PATCH 1/5] refactor to allow exporting rich contents to formatted string (html, svg, ...) --- databooks/tui.py | 114 ++++++++++++++++++++++++++++++++++++++++------- pyproject.toml | 1 + 2 files changed, 100 insertions(+), 15 deletions(-) diff --git a/databooks/tui.py b/databooks/tui.py index 5124f415..38639124 100644 --- a/databooks/tui.py +++ b/databooks/tui.py @@ -1,8 +1,9 @@ """Terminal user interface (TUI) helper functions and components.""" -from contextlib import nullcontext +from contextlib import AbstractContextManager, nullcontext from dataclasses import asdict +from enum import Enum from pathlib import Path -from typing import List +from typing import Any, Dict, List, Optional, Union, overload from rich.columns import Columns from rich.console import Console @@ -14,30 +15,75 @@ DATABOOKS_TUI = Theme({"in_count": "blue", "out_count": "orange3", "kernel": "bold"}) -databooks_console = Console(theme=DATABOOKS_TUI) +ImgFmt = Enum("ImgFmt", {"html": "HTML", "svg": "SVG", "text": "TXT"}) -def print_nb(path: Path, console: Console = databooks_console) -> None: +def print_nb( + path: Path, + console: Console = Console(), +) -> None: """Show rich representation of notebook in terminal.""" notebook = JupyterNotebook.parse_file(path) - console.rule(path.resolve().name) - console.print(notebook) + console.print(Rule(path.resolve().name), notebook) + + +@overload +def print_nbs( + paths: List[Path], + *, + context: ImgFmt, + **kwargs: Any, +) -> str: + ... +@overload def print_nbs( paths: List[Path], - console: Console = databooks_console, - use_pager: bool = False, + *, + context: bool, + **kwargs: Any, ) -> None: - """Show rich representation of notebooks in terminal.""" - with console.pager(styles=True) if use_pager else nullcontext(): # type: ignore + ... + + +def print_nbs( + paths: List[Path], + *, + context: Union[ImgFmt, bool] = False, + export_kwargs: Optional[Dict[str, Any]] = None, + **console_kwargs: Any, +) -> Optional[str]: + """ + Show rich representation of notebooks in terminal. + + :param paths: notebook paths to print + :param context: specify context - `ImgFmt` to export outputs, `True` for `pager` + :param export_kwargs: keyword arguments for exporting prints (as a dictionary) + :param console_kwargs: keyword arguments to be passed to `Console` + :return: console output if `context` is `ImgFmt`, else `None` + """ + if "record" in console_kwargs: + raise ValueError( + "Specify `record` parameter of console via `context` argument." + ) + theme = console_kwargs.pop("theme", DATABOOKS_TUI) + console = Console(record=isinstance(context, ImgFmt), theme=theme, **console_kwargs) + ctx_map: Dict[Union[ImgFmt, bool], AbstractContextManager] = { + True: console.pager(styles=True), + False: nullcontext(), + } + with ctx_map.get(context, console.capture()): for path in paths: print_nb(path, console=console) + if isinstance(context, ImgFmt): + return getattr(console, f"export_{context.name}")(**(export_kwargs or {})) def print_diff( diff: DiffContents, - console: Console = databooks_console, + *, + console: Console = Console(), ) -> None: """Show rich representation of notebook diff in terminal.""" a_nb, b_nb = ( @@ -62,12 +108,50 @@ def print_diff( console.print(cols, a_nb - b_nb) +@overload def print_diffs( diffs: List[DiffContents], - console: Console = databooks_console, - use_pager: bool = False, + *, + context: ImgFmt, + **kwargs: Any, +) -> str: + ... + + +@overload +def print_diffs( + diffs: List[DiffContents], + *, + context: bool, + **kwargs: Any, ) -> None: - """Show rich representation of notebook diff in terminal.""" - with console.pager(styles=True) if use_pager else nullcontext(): # type: ignore + ... + + +def print_diffs( + diffs: List[DiffContents], + *, + context: Union[ImgFmt, bool] = False, + export_kwargs: Optional[Dict[str, Any]] = None, + **console_kwargs: Any, +) -> Optional[str]: + """ + Show rich representation of notebook diff in terminal. + + :param diffs: `databooks.git_utils.DiffContents` for rendering + :param context: specify context - `ImgFmt` to export outputs, `True` for `pager` + :param export_kwargs: keyword arguments for exporting prints (as a dictionary) + :param console_kwargs: keyword arguments to be passed to `Console` + :return: console output if `context` is `ImgFmt`, else `None` + """ + theme = console_kwargs.pop("theme", DATABOOKS_TUI) + console = Console(record=isinstance(context, ImgFmt), theme=theme, **console_kwargs) + ctx_map: Dict[Union[ImgFmt, bool], AbstractContextManager] = { + True: console.pager(styles=True), + False: nullcontext(), + } + with ctx_map.get(context, console.capture()): for diff in diffs: print_diff(diff, console=console) + if isinstance(context, ImgFmt): + return getattr(console, f"export_{context.name}")(**(export_kwargs or {})) diff --git a/pyproject.toml b/pyproject.toml index 7e46025f..f5bdfd2b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ show_error_codes = true disallow_untyped_calls = true disallow_untyped_defs = true ignore_missing_imports = true +warn_no_return = false [tool.isort] skip_glob = ["docs/*"] From 1544411577fdbb8d7394f89180a7aba90172ffa6 Mon Sep 17 00:00:00 2001 From: Murilo Cunha Date: Fri, 11 Nov 2022 17:26:37 +0100 Subject: [PATCH 2/5] minor fixes on tests (newlines) --- tests/test_cli.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index befac6b4..767aca6e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -456,6 +456,7 @@ def test_show() -> None: ╭──────────────────────────────────────────────────────────────────────────────╮ │ │ ╰──────────────────────────────────────────────────────────────────────────────╯ + """ ) @@ -527,6 +528,7 @@ def test_diff(tmp_path: Path) -> None: ╭─────────────────────────────────────╮ │ extra │ ╰─────────────────────────────────────╯ + """ ) @@ -556,5 +558,6 @@ def test_diff(tmp_path: Path) -> None: ╭─────────────────────────────────────╮ │ extra │ ╰─────────────────────────────────────╯ + """ ) From ed5c5a210d46c125da7e49a99febe8f865ec97b5 Mon Sep 17 00:00:00 2001 From: Murilo Cunha Date: Fri, 11 Nov 2022 17:28:58 +0100 Subject: [PATCH 3/5] add cli functionality to export html, svg and txt --- databooks/cli.py | 28 ++++++++++++++++++++++++---- databooks/git_utils.py | 1 - docs/CLI.md | 2 ++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/databooks/cli.py b/databooks/cli.py index f8fddd00..524651e5 100644 --- a/databooks/cli.py +++ b/databooks/cli.py @@ -22,7 +22,7 @@ from databooks.logging import get_logger from databooks.metadata import clear_all from databooks.recipes import Recipe -from databooks.tui import print_diffs, print_nbs +from databooks.tui import ImgFmt, print_diffs, print_nbs from databooks.version import __version__ logger = get_logger(__file__) @@ -381,6 +381,12 @@ def show( ..., is_eager=True, help="Path(s) of notebook files with conflicts" ), ignore: List[str] = Option(["!*"], help="Glob expression(s) of files to ignore"), + export: Optional[ImgFmt] = Option( + None, + "--export", + "-x", + help="Export rich outputs as a string.", + ), pager: bool = Option( False, "--pager", "-p", help="Use pager instead of printing to terminal" ), @@ -407,12 +413,18 @@ def show( ), ) -> None: """Show rich representation of notebook.""" + if export is not None and pager: + raise BadParameter("Cannot use both pager and export output.") nb_paths = _check_paths(paths=paths, ignore=ignore) if len(nb_paths) > 1 and not multiple: if not Confirm.ask(f"Show {len(nb_paths)} notebooks?"): raise Exit() - - print_nbs(nb_paths, use_pager=pager) + echo( + print_nbs( + nb_paths, + context=export or pager, + ) + ) @app.command() @@ -427,6 +439,12 @@ def diff( None, is_eager=True, help="Path(s) of notebook files to compare" ), ignore: List[str] = Option(["!*"], help="Glob expression(s) of files to ignore"), + export: Optional[ImgFmt] = Option( + None, + "--export", + "-x", + help="Export rich outputs as a string.", + ), pager: bool = Option( False, "--pager", "-p", help="Use pager instead of printing to terminal" ), @@ -460,6 +478,8 @@ def diff( means we can compare files that are staged with other branches, hashes, etc., or compare the current directory with the current index. """ + if export is not None and pager: + raise BadParameter("Cannot use both pager and export output.") (ref_base, ref_remote), paths = _parse_paths(ref_base, ref_remote, paths=paths) diffs = get_nb_diffs( ref_base=ref_base, ref_remote=ref_remote, paths=paths, verbose=verbose @@ -470,4 +490,4 @@ def diff( if len(diffs) > 1 and not multiple: if not Confirm.ask(f"Show {len(diffs)} notebook diffs?"): raise Exit() - print_diffs(diffs=diffs, use_pager=pager) + echo(print_diffs(diffs=diffs, context=export or pager)) diff --git a/databooks/git_utils.py b/databooks/git_utils.py index 43ed79e6..9002a257 100644 --- a/databooks/git_utils.py +++ b/databooks/git_utils.py @@ -120,7 +120,6 @@ def get_repo(path: Path) -> Optional[Repo]: return repo else: logger.debug(f"No repo found at {path}.") - return None def get_conflict_blobs(repo: Repo) -> List[ConflictFile]: diff --git a/docs/CLI.md b/docs/CLI.md index 141ebb02..8201bcb4 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -101,6 +101,7 @@ $ databooks diff [OPTIONS] [REF_BASE] [REF_REMOTE] [PATHS]... **Options**: * `--ignore TEXT`: Glob expression(s) of files to ignore [default: !*] +* `-x, --export [HTML|SVG|TXT]`: Export rich outputs as a string. * `-p, --pager`: Use pager instead of printing to terminal [default: False] * `-v, --verbose`: Increase verbosity for debugging [default: False] * `-y, --yes`: Show multiple files [default: False] @@ -183,6 +184,7 @@ $ databooks show [OPTIONS] PATHS... **Options**: * `--ignore TEXT`: Glob expression(s) of files to ignore [default: !*] +* `-x, --export [HTML|SVG|TXT]`: Export rich outputs as a string. * `-p, --pager`: Use pager instead of printing to terminal [default: False] * `-v, --verbose`: Increase verbosity for debugging [default: False] * `-y, --yes`: Show multiple files [default: False] From 88b81e8622cc566d886f2493ed316ed99635ad8f Mon Sep 17 00:00:00 2001 From: Murilo Cunha Date: Fri, 11 Nov 2022 17:43:16 +0100 Subject: [PATCH 4/5] add test for export --- tests/test_cli.py | 86 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index 767aca6e..d5b0371c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -561,3 +561,89 @@ def test_diff(tmp_path: Path) -> None: """ ) + + +def test_diff_svg(tmp_path: Path) -> None: + """Show rich diffs of notebooks.""" + nb_path = Path("test_conflicts_nb.ipynb") + notebook_1 = TestJupyterNotebook().jupyter_notebook + notebook_2 = TestJupyterNotebook().jupyter_notebook + + notebook_1.metadata = NotebookMetadata( + kernelspec=dict( + display_name="different_kernel_display_name", name="kernel_name" + ), + field_to_remove=["Field to remove"], + another_field_to_remove="another field", + ) + + extra_cell = BaseCell( + cell_type="raw", + metadata=CellMetadata(random_meta=["meta"]), + source="extra", + ) + notebook_1.cells = notebook_1.cells + [extra_cell] + notebook_2.nbformat += 1 + notebook_2.nbformat_minor += 1 + + _ = init_repo_diff( + tmp_path=tmp_path, + filename=nb_path, + contents_main=notebook_1.json(), + contents_other=notebook_2.json(), + commit_message_main="Notebook from main", + commit_message_other="Notebook from other", + ) + + # Test passing another branch to compare + result = runner.invoke(app, ["diff", "other", str(tmp_path), "-x", "HTML"]) + assert ( + result.output + == """\ + + + + + + + + + 
────── a/test_conflicts_nb.ipynb\
+ ───────────── b/test_conflicts_nb.ipynb ───────
+                     kernel_display_name \
+          different_kernel_display_name
+In [1]:
+╭──────────────────────────────────────────────────────────────────────────────╮
+│ test_source                          \
+                                        │
+╰──────────────────────────────────────────────────────────────────────────────╯
+test text
+
+In [1]:
+╭──────────────────────────────────────────────────────────────────────────────╮
+│ test_source                          \
+                                        │
+╰──────────────────────────────────────────────────────────────────────────────╯
+test text
+
+                                         ╭─────────────────────────────────────╮
+                 <None>                  │ extra                               │
+                                         ╰─────────────────────────────────────╯
+
+ + + + +""" + ) From 39badd2c9cbdfa9a3174447948e6d65d78cb810f Mon Sep 17 00:00:00 2001 From: Murilo Cunha Date: Fri, 11 Nov 2022 19:43:52 +0100 Subject: [PATCH 5/5] rename tui functions --- databooks/cli.py | 6 +++--- databooks/tui.py | 20 ++++++++++---------- tests/test_tui.py | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/databooks/cli.py b/databooks/cli.py index 524651e5..e21ee3ec 100644 --- a/databooks/cli.py +++ b/databooks/cli.py @@ -22,7 +22,7 @@ from databooks.logging import get_logger from databooks.metadata import clear_all from databooks.recipes import Recipe -from databooks.tui import ImgFmt, print_diffs, print_nbs +from databooks.tui import ImgFmt, diffs2rich, nbs2rich from databooks.version import __version__ logger = get_logger(__file__) @@ -420,7 +420,7 @@ def show( if not Confirm.ask(f"Show {len(nb_paths)} notebooks?"): raise Exit() echo( - print_nbs( + nbs2rich( nb_paths, context=export or pager, ) @@ -490,4 +490,4 @@ def diff( if len(diffs) > 1 and not multiple: if not Confirm.ask(f"Show {len(diffs)} notebook diffs?"): raise Exit() - echo(print_diffs(diffs=diffs, context=export or pager)) + echo(diffs2rich(diffs=diffs, context=export or pager)) diff --git a/databooks/tui.py b/databooks/tui.py index 38639124..3a77648e 100644 --- a/databooks/tui.py +++ b/databooks/tui.py @@ -18,7 +18,7 @@ ImgFmt = Enum("ImgFmt", {"html": "HTML", "svg": "SVG", "text": "TXT"}) -def print_nb( +def nb2rich( path: Path, console: Console = Console(), ) -> None: @@ -28,7 +28,7 @@ def print_nb( @overload -def print_nbs( +def nbs2rich( paths: List[Path], *, context: ImgFmt, @@ -38,7 +38,7 @@ def print_nbs( @overload -def print_nbs( +def nbs2rich( paths: List[Path], *, context: bool, @@ -47,7 +47,7 @@ def print_nbs( ... -def print_nbs( +def nbs2rich( paths: List[Path], *, context: Union[ImgFmt, bool] = False, @@ -75,12 +75,12 @@ def print_nbs( } with ctx_map.get(context, console.capture()): for path in paths: - print_nb(path, console=console) + nb2rich(path, console=console) if isinstance(context, ImgFmt): return getattr(console, f"export_{context.name}")(**(export_kwargs or {})) -def print_diff( +def diff2rich( diff: DiffContents, *, console: Console = Console(), @@ -109,7 +109,7 @@ def print_diff( @overload -def print_diffs( +def diffs2rich( diffs: List[DiffContents], *, context: ImgFmt, @@ -119,7 +119,7 @@ def print_diffs( @overload -def print_diffs( +def diffs2rich( diffs: List[DiffContents], *, context: bool, @@ -128,7 +128,7 @@ def print_diffs( ... -def print_diffs( +def diffs2rich( diffs: List[DiffContents], *, context: Union[ImgFmt, bool] = False, @@ -152,6 +152,6 @@ def print_diffs( } with ctx_map.get(context, console.capture()): for diff in diffs: - print_diff(diff, console=console) + diff2rich(diff, console=console) if isinstance(context, ImgFmt): return getattr(console, f"export_{context.name}")(**(export_kwargs or {})) diff --git a/tests/test_tui.py b/tests/test_tui.py index 288717bb..706c7c1d 100644 --- a/tests/test_tui.py +++ b/tests/test_tui.py @@ -6,7 +6,7 @@ from databooks.data_models.cell import CellMetadata, RawCell from databooks.data_models.notebook import JupyterNotebook, NotebookMetadata -from databooks.tui import print_nb +from databooks.tui import nb2rich from tests.test_data_models.test_notebook import TestJupyterNotebook with resources.path("tests.files", "tui-demo.ipynb") as nb_path: @@ -193,7 +193,7 @@ def test_print_nb() -> None: """Print notebook from path and add rules with file name.""" console = Console(file=io.StringIO(), width=50, legacy_windows=False) with resources.path("tests.files", "tui-demo.ipynb") as path: - print_nb(path, console=console) + nb2rich(path, console=console) assert console.file.getvalue() == "\n".join( ("───────────────── tui-demo.ipynb ─────────────────", rich_nb) )