Refactor to put rich output formatting in the app namespace

servicex/app/main.py

@@ -63,6 +63,46 @@ def show_version(show: bool):
         raise typer.Exit()
 
 
+def _display_results(out_dict):
+    """Display the delivery results using rich styling."""
+    from rich import get_console
+
+    console = get_console()
+
+    console.print("\n[bold green]✓ ServiceX Delivery Complete![/bold green]\n")
+
+    table = rich.table.Table(
+        title="Delivered Files", show_header=True, header_style="bold magenta"
+    )
+    table.add_column("Sample", style="cyan", no_wrap=True)
+    table.add_column("File Count", justify="right", style="green")
+    table.add_column("Files", style="dim")
+
+    total_files = 0
+    for sample_name, files in out_dict.items():
+        if isinstance(files, servicex_client.GuardList) and files.valid():
+            file_list = list(files)
+            file_count = len(file_list)
+            total_files += file_count
+
+            # Show first few files with ellipsis if many
+            if file_count <= 3:
+                files_display = "\n".join(str(f) for f in file_list)
+            else:
+                files_display = "\n".join(str(f) for f in file_list[:2])
+                files_display += f"\n... and {file_count - 2} more files"
+
+            table.add_row(sample_name, str(file_count), files_display)
+        else:
+            # Handle error case
+            table.add_row(
+                sample_name, "[red]Error[/red]", "[red]Failed to retrieve files[/red]"
+            )
+
+    console.print(table)
+    console.print(f"\n[bold blue]Total files delivered: {total_files}[/bold blue]\n")
+
+
 version_opt = typer.Option(None, "--version", callback=show_version, is_eager=True)
 
 
@@ -87,13 +127,14 @@ def deliver(
     """
 
     print(f"Delivering {spec_file} to ServiceX cache")
-    servicex_client.deliver(
+    results = servicex_client.deliver(
         spec_file,
         servicex_name=backend,
         config_path=config_path,
         ignore_local_cache=ignore_cache,
-        display_results=not hide_results,
     )
+    if not hide_results:
+        _display_results(results)
 
 
 if __name__ == "__main__":

servicex/servicex_client.py

@@ -52,7 +52,6 @@
 from collections.abc import Sequence, Coroutine
 from enum import Enum
 import traceback
-from rich.table import Table
 
 T = TypeVar("T")
 logger = logging.getLogger(__name__)
@@ -246,46 +245,6 @@ def _get_progress_options(progress_bar: ProgressBarFormat) -> dict:
         raise ValueError(f"Invalid value {progress_bar} for progress_bar provided")
 
 
-def _display_results(out_dict):
-    """Display the delivery results using rich styling."""
-    from rich import get_console
-
-    console = get_console()
-
-    console.print("\n[bold green]✓ ServiceX Delivery Complete![/bold green]\n")
-
-    table = Table(
-        title="Delivered Files", show_header=True, header_style="bold magenta"
-    )
-    table.add_column("Sample", style="cyan", no_wrap=True)
-    table.add_column("File Count", justify="right", style="green")
-    table.add_column("Files", style="dim")
-
-    total_files = 0
-    for sample_name, files in out_dict.items():
-        if isinstance(files, GuardList) and files.valid():
-            file_list = list(files)
-            file_count = len(file_list)
-            total_files += file_count
-
-            # Show first few files with ellipsis if many
-            if file_count <= 3:
-                files_display = "\n".join(str(f) for f in file_list)
-            else:
-                files_display = "\n".join(str(f) for f in file_list[:2])
-                files_display += f"\n... and {file_count - 2} more files"
-
-            table.add_row(sample_name, str(file_count), files_display)
-        else:
-            # Handle error case
-            table.add_row(
-                sample_name, "[red]Error[/red]", "[red]Failed to retrieve files[/red]"
-            )
-
-    console.print(table)
-    console.print(f"\n[bold blue]Total files delivered: {total_files}[/bold blue]\n")
-
-
 async def deliver_async(
     spec: Union[ServiceXSpec, Mapping[str, Any], str, Path],
     config_path: Optional[str] = None,
@@ -294,7 +253,6 @@ async def deliver_async(
     fail_if_incomplete: bool = True,
     ignore_local_cache: bool = False,
     progress_bar: ProgressBarFormat = ProgressBarFormat.default,
-    display_results: bool = True,
     concurrency: int = 10,
 ):
     r"""
@@ -317,8 +275,6 @@ async def deliver_async(
             will have its own progress bars; :py:const:`ProgressBarFormat.compact` gives one
             summary progress bar for all transformations; :py:const:`ProgressBarFormat.none`
             switches off progress bars completely.
-    :param display_results: Specifies whether the results should be displayed to the console.
-            Defaults to True.
     :param concurrency: specify how many downloads to run in parallel (default is 8).
     :return: A dictionary mapping the name of each :py:class:`Sample` to a :py:class:`.GuardList`
             with the file names or URLs for the outputs.
@@ -360,9 +316,6 @@ async def deliver_async(
 
     output_dict = _output_handler(config, datasets, results)
 
-    if display_results:
-        _display_results(output_dict)
-
     return output_dict
 
 

tests/app/test_app.py

@@ -30,6 +30,7 @@
 from datetime import datetime, timezone
 from pathlib import Path
 from unittest.mock import Mock, patch
+from servicex.servicex_client import GuardList
 
 from servicex.models import ResultFormat, TransformedResults
 
@@ -43,42 +44,34 @@ def test_app_version(script_runner):
 
 
 def test_deliver(script_runner):
-    with patch("servicex.app.main.servicex_client") as mock_servicex_client:
-        mock_servicex_client.deliver = Mock(
-            return_value={"UprootRaw_YAML": ["/tmp/foo.root", "/tmp/bar.root"]}
-        )
+    with patch("servicex.app.main.servicex_client.deliver") as mock_servicex_client:
+        mock_servicex_client.return_value = {
+            "UprootRaw_YAML": GuardList(["/tmp/foo.root", "/tmp/bar.root"])
+        }
         result = script_runner.run(["servicex", "deliver", "foo.yaml"])
         assert result.returncode == 0
         result_rows = result.stdout.split("\n")
         assert result_rows[0] == "Delivering foo.yaml to ServiceX cache"
-        mock_servicex_client.deliver.assert_called_once_with(
+        mock_servicex_client.assert_called_once_with(
             "foo.yaml",
             servicex_name=None,
             config_path=None,
             ignore_local_cache=None,
-            display_results=True,
         )
+        assert result_rows[-3] == "Total files delivered: 2"
 
 
 def test_deliver_hide_results(script_runner):
-    with patch("servicex.app.main.servicex_client") as mock_servicex_client:
-        mock_servicex_client.deliver = Mock(
-            return_value={"UprootRaw_YAML": ["/tmp/foo.root", "/tmp/bar.root"]}
-        )
+    with patch("servicex.app.main.servicex_client.deliver") as mock_servicex_client:
+        mock_servicex_client.return_value = {
+            "UprootRaw_YAML": ["/tmp/foo.root", "/tmp/bar.root"]
+        }
         result = script_runner.run(
             ["servicex", "deliver", "foo.yaml", "--hide-results"]
         )
         assert result.returncode == 0
         result_rows = result.stdout.split("\n")
         assert result_rows[0] == "Delivering foo.yaml to ServiceX cache"
-        # Verify that servicex_client.deliver was called with display_results=False
-        mock_servicex_client.deliver.assert_called_once_with(
-            "foo.yaml",
-            servicex_name=None,
-            config_path=None,
-            ignore_local_cache=None,
-            display_results=False,
-        )
 
 
 def test_cache_list(script_runner, tmp_path) -> None:

tests/test_display_results.py

@@ -30,11 +30,11 @@
 
 from servicex.servicex_client import (
     GuardList,
-    _display_results,
     _get_progress_options,
     ProgressBarFormat,
     ServiceXClient,
 )
+from servicex.app.main import _display_results
 
 
 def test_display_results_basic():
@@ -50,7 +50,7 @@ def test_display_results_basic():
         mock_console = Mock()
         mock_get_console.return_value = mock_console
 
-        with patch("servicex.servicex_client.Table"):
+        with patch("rich.table.Table"):
             _display_results(out_dict)
 
             # Just verify it was called - don't over-test internal details
@@ -90,3 +90,40 @@ def test_guardlist_basics():
     assert not error_list.valid()
     with pytest.raises(ReturnValueException):
         _ = error_list[0]
+
+
+def test_display_results_with_many_files():
+    from servicex.servicex_client import GuardList
+    from unittest.mock import patch, MagicMock
+
+    # Mock GuardList with more than 3 files to trigger lines 275-276
+    mock_guard_list = MagicMock(spec=GuardList)
+    mock_guard_list.valid.return_value = True
+    mock_guard_list.__iter__.return_value = iter(
+        [
+            "file1.parquet",
+            "file2.parquet",
+            "file3.parquet",
+            "file4.parquet",
+            "file5.parquet",
+        ]
+    )
+
+    out_dict = {"sample1": mock_guard_list}
+
+    with patch("rich.get_console") as mock_get_console:
+        mock_console = MagicMock()
+        mock_get_console.return_value = mock_console
+
+        with patch("rich.table.Table") as mock_table:
+            mock_table_instance = MagicMock()
+            mock_table.return_value = mock_table_instance
+
+            _display_results(out_dict)
+
+            # Verify that add_row was called with the truncated file list
+            mock_table_instance.add_row.assert_called_once()
+            call_args = mock_table_instance.add_row.call_args[0]
+            assert call_args[0] == "sample1"
+            assert call_args[1] == "5"
+            assert "... and 3 more files" in call_args[2]

tests/test_servicex_client.py

@@ -151,43 +151,6 @@ def test_invalid_backend_raises_error_with_filename():
     assert f"Backend badname not defined in {expected} file" in str(err.value)
 
 
-def test_display_results_with_many_files():
-    from servicex.servicex_client import _display_results, GuardList
-    from unittest.mock import patch, MagicMock
-
-    # Mock GuardList with more than 3 files to trigger lines 275-276
-    mock_guard_list = MagicMock(spec=GuardList)
-    mock_guard_list.valid.return_value = True
-    mock_guard_list.__iter__.return_value = iter(
-        [
-            "file1.parquet",
-            "file2.parquet",
-            "file3.parquet",
-            "file4.parquet",
-            "file5.parquet",
-        ]
-    )
-
-    out_dict = {"sample1": mock_guard_list}
-
-    with patch("rich.get_console") as mock_get_console:
-        mock_console = MagicMock()
-        mock_get_console.return_value = mock_console
-
-        with patch("servicex.servicex_client.Table") as mock_table:
-            mock_table_instance = MagicMock()
-            mock_table.return_value = mock_table_instance
-
-            _display_results(out_dict)
-
-            # Verify that add_row was called with the truncated file list
-            mock_table_instance.add_row.assert_called_once()
-            call_args = mock_table_instance.add_row.call_args[0]
-            assert call_args[0] == "sample1"
-            assert call_args[1] == "5"
-            assert "... and 3 more files" in call_args[2]
-
-
 @pytest.mark.asyncio
 async def test_deliver_async_invalid_delivery_config():
     from servicex.servicex_client import deliver_async