From 6a688147d462c7e79726d2a49ea48560955c6058 Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Tue, 14 Jan 2025 23:57:33 +0100 Subject: [PATCH] fix: Remove icon from rich representation to have a cleaner display (#1115) This PR is removing the icon used in the different `help` method such that we don't have issue with spacing. https://github.com/probabl-ai/skore/issues/1104 will provide a richer output where we can leverage those types of icons instead to purely rely on textual that are grant in terminal mainly. --- skore/src/skore/sklearn/_base.py | 15 +++++---------- skore/src/skore/sklearn/_base.pyi | 3 +-- .../skore/sklearn/_estimator/metrics_accessor.py | 10 +++++----- skore/src/skore/sklearn/_estimator/report.py | 5 +---- skore/src/skore/sklearn/_plot/utils.py | 2 +- skore/tests/unit/sklearn/plot/test_common.py | 2 +- skore/tests/unit/sklearn/test_base.py | 10 +++++----- skore/tests/unit/sklearn/test_estimator.py | 8 +++----- 8 files changed, 22 insertions(+), 33 deletions(-) diff --git a/skore/src/skore/sklearn/_base.py b/skore/src/skore/sklearn/_base.py index bd043023b..a9a262a9e 100644 --- a/skore/src/skore/sklearn/_base.py +++ b/skore/src/skore/sklearn/_base.py @@ -87,7 +87,7 @@ def _rich_repr(self, class_name, help_method_name): class _BaseReport(_HelpMixin): def _get_help_panel_title(self): return ( - f"[bold cyan]:notebook: Tools to diagnose estimator " + f"[bold cyan]Tools to diagnose estimator " f"{self.estimator_name}[/bold cyan]" ) @@ -132,9 +132,7 @@ def _create_help_tree(self): # Add accessor methods first for accessor_attr, config in self._ACCESSOR_CONFIG.items(): accessor = getattr(self, accessor_attr) - branch = tree.add( - f"[bold cyan].{config['name']} {config['icon']}[/bold cyan]" - ) + branch = tree.add(f"[bold cyan].{config['name']}[/bold cyan]") # Add main accessor methods first methods = accessor._get_methods_for_help() @@ -149,9 +147,7 @@ def _create_help_tree(self): # Add sub-accessors after main methods for sub_attr, sub_obj in inspect.getmembers(accessor): if isinstance(sub_obj, _BaseAccessor) and not sub_attr.startswith("_"): - sub_branch = branch.add( - f"[bold cyan].{sub_attr} {sub_obj._icon}[/bold cyan]" - ) + sub_branch = branch.add(f"[bold cyan].{sub_attr}[/bold cyan]") # Add sub-accessor methods sub_methods = sub_obj._get_methods_for_help() @@ -183,13 +179,12 @@ def _create_help_tree(self): class _BaseAccessor(_HelpMixin): """Base class for all accessors.""" - def __init__(self, parent, icon): + def __init__(self, parent): self._parent = parent - self._icon = icon def _get_help_panel_title(self): name = self.__class__.__name__.replace("_", "").replace("Accessor", "").lower() - return f"{self._icon} Available {name} methods" + return f"Available {name} methods" def _create_help_tree(self): """Create a rich Tree with the available methods.""" diff --git a/skore/src/skore/sklearn/_base.pyi b/skore/src/skore/sklearn/_base.pyi index d1c7fc826..155db3d05 100644 --- a/skore/src/skore/sklearn/_base.pyi +++ b/skore/src/skore/sklearn/_base.pyi @@ -20,9 +20,8 @@ class _HelpMixin: class _BaseAccessor(_HelpMixin): _parent: Any - _icon: str - def __init__(self, parent: Any, icon: str) -> None: ... + def __init__(self, parent: Any) -> None: ... def _get_help_panel_title(self) -> str: ... def _create_help_tree(self) -> Tree: ... def _get_X_y_and_data_source_hash( diff --git a/skore/src/skore/sklearn/_estimator/metrics_accessor.py b/skore/src/skore/sklearn/_estimator/metrics_accessor.py index fc741b672..0a84379bf 100644 --- a/skore/src/skore/sklearn/_estimator/metrics_accessor.py +++ b/skore/src/skore/sklearn/_estimator/metrics_accessor.py @@ -42,7 +42,7 @@ class _MetricsAccessor(_BaseAccessor, DirNamesMixin): } def __init__(self, parent): - super().__init__(parent, icon=":straight_ruler:") + super().__init__(parent) # TODO: should build on the `add_scorers` function def report_metrics( @@ -797,7 +797,7 @@ def _create_help_tree(self): tree = super()._create_help_tree() # Add plot methods in a separate branch - plot_branch = tree.add("[bold cyan].plot :art:[/bold cyan]") + plot_branch = tree.add("[bold cyan].plot[/bold cyan]") plot_methods = self.plot._get_methods_for_help() plot_methods = self.plot._sort_methods_for_help(plot_methods) @@ -809,7 +809,7 @@ def _create_help_tree(self): return tree def _get_help_panel_title(self): - return f"[bold cyan]{self._icon} Available metrics methods[/bold cyan]" + return "[bold cyan]Available metrics methods[/bold cyan]" def _get_help_legend(self): return ( @@ -837,7 +837,7 @@ class _PlotMetricsAccessor(_BaseAccessor): """Plotting methods for the metrics accessor.""" def __init__(self, parent): - super().__init__(parent._parent, icon=":art:") + super().__init__(parent._parent) self._metrics_parent = parent def _get_display( @@ -1099,7 +1099,7 @@ def prediction_error( ) def _get_help_panel_title(self): - return f"[bold cyan]{self._icon} Available plot methods[/bold cyan]" + return "[bold cyan]Available plot methods[/bold cyan]" def _get_help_tree_title(self): return "[bold cyan]reporter.metrics.plot[/bold cyan]" diff --git a/skore/src/skore/sklearn/_estimator/report.py b/skore/src/skore/sklearn/_estimator/report.py index a1cce9f74..3e71f7986 100644 --- a/skore/src/skore/sklearn/_estimator/report.py +++ b/skore/src/skore/sklearn/_estimator/report.py @@ -64,10 +64,7 @@ class EstimatorReport(_BaseReport, DirNamesMixin): """ _ACCESSOR_CONFIG = { - "metrics": {"icon": ":straight_ruler:", "name": "metrics"}, - # Add other accessors as they're implemented - # "inspection": {"icon": ":magnifying_glass:", "name": "inspection"}, - # "linting": {"icon": ":check:", "name": "linting"}, + "metrics": {"name": "metrics"}, } @staticmethod diff --git a/skore/src/skore/sklearn/_plot/utils.py b/skore/src/skore/sklearn/_plot/utils.py index c8bc39f1f..7aa4b1e0b 100644 --- a/skore/src/skore/sklearn/_plot/utils.py +++ b/skore/src/skore/sklearn/_plot/utils.py @@ -66,7 +66,7 @@ def _create_help_panel(self): return Panel( self._create_help_tree(), title=( - f"[bold cyan]:bar_chart: {self.__class__.__name__} for " + f"[bold cyan]{self.__class__.__name__} for " f"{self.estimator_name}[/bold cyan]" ), border_style="orange1", diff --git a/skore/tests/unit/sklearn/plot/test_common.py b/skore/tests/unit/sklearn/plot/test_common.py index 26d595777..9f0d33824 100644 --- a/skore/tests/unit/sklearn/plot/test_common.py +++ b/skore/tests/unit/sklearn/plot/test_common.py @@ -28,7 +28,7 @@ def test_display_help(pyplot, capsys, plot_func, estimator, dataset): display.help() captured = capsys.readouterr() - assert f"📊 {display.__class__.__name__}" in captured.out + assert f"{display.__class__.__name__}" in captured.out @pytest.mark.parametrize( diff --git a/skore/tests/unit/sklearn/test_base.py b/skore/tests/unit/sklearn/test_base.py index a183b2922..0f6b12b91 100644 --- a/skore/tests/unit/sklearn/test_base.py +++ b/skore/tests/unit/sklearn/test_base.py @@ -213,7 +213,7 @@ def test_base_accessor_get_X_y_and_data_source_hash_error(): estimator = LogisticRegression().fit(X_train, y_train) report = MockReport(estimator, X_train=None, y_train=None, X_test=None, y_test=None) - accessor = _BaseAccessor(parent=report, icon="") + accessor = _BaseAccessor(parent=report) err_msg = re.escape( "Invalid data source: unknown. Possible values are: " "test, train, X_y." @@ -234,7 +234,7 @@ def test_base_accessor_get_X_y_and_data_source_hash_error(): report = MockReport( estimator, X_train=X_train, y_train=y_train, X_test=X_test, y_test=y_test ) - accessor = _BaseAccessor(parent=report, icon="") + accessor = _BaseAccessor(parent=report) for data_source in ("train", "test"): err_msg = f"X and y must be None when data_source is {data_source}." @@ -251,13 +251,13 @@ def test_base_accessor_get_X_y_and_data_source_hash_error(): # use `custom_metric` for them. estimator = KMeans(n_clusters=2).fit(X_train) report = MockReport(estimator, X_test=X_test) - accessor = _BaseAccessor(parent=report, icon="") + accessor = _BaseAccessor(parent=report) err_msg = "X must be provided." with pytest.raises(ValueError, match=err_msg): accessor._get_X_y_and_data_source_hash(data_source="X_y") report = MockReport(estimator) - accessor = _BaseAccessor(parent=report, icon="") + accessor = _BaseAccessor(parent=report) for data_source in ("train", "test"): err_msg = re.escape( f"No {data_source} data (i.e. X_{data_source}) were provided when " @@ -279,7 +279,7 @@ def test_base_accessor_get_X_y_and_data_source_hash(data_source): report = MockReport( estimator, X_train=X_train, y_train=y_train, X_test=X_test, y_test=y_test ) - accessor = _BaseAccessor(parent=report, icon="") + accessor = _BaseAccessor(parent=report) kwargs = {"X": X_test, "y": y_test} if data_source == "X_y" else {} X, y, data_source_hash = accessor._get_X_y_and_data_source_hash( data_source=data_source, **kwargs diff --git a/skore/tests/unit/sklearn/test_estimator.py b/skore/tests/unit/sklearn/test_estimator.py index 05d642926..fa55dc2de 100644 --- a/skore/tests/unit/sklearn/test_estimator.py +++ b/skore/tests/unit/sklearn/test_estimator.py @@ -274,9 +274,7 @@ def test_estimator_report_help(capsys, binary_classification_data): report.help() captured = capsys.readouterr() - assert ( - f"📓 Tools to diagnose estimator {estimator.__class__.__name__}" in captured.out - ) + assert f"Tools to diagnose estimator {estimator.__class__.__name__}" in captured.out def test_estimator_report_repr(binary_classification_data): @@ -336,7 +334,7 @@ def test_estimator_report_plot_help(capsys, binary_classification_data): report.metrics.plot.help() captured = capsys.readouterr() - assert "🎨 Available plot methods" in captured.out + assert "Available plot methods" in captured.out def test_estimator_report_plot_repr(binary_classification_data): @@ -476,7 +474,7 @@ def test_estimator_report_metrics_help(capsys, binary_classification_data): report.metrics.help() captured = capsys.readouterr() - assert "📏 Available metrics methods" in captured.out + assert "Available metrics methods" in captured.out def test_estimator_report_metrics_repr(binary_classification_data):