refactor(progress_decorator): Make sure self_obj is a Report (#1536)

This originates from a bug when implementing comparison of
CrossValidationReport in #1512

Because CrossValidationReports hold a `_parent_progress`, when a
ComparisonReport creates a progress bar to iterate over
CrossValidationReports, the outer progress bar conflicts with the inner
progress bars, and rich refuses to proceed.

The solution is for the ComparisonReport to explicitly set its inner
CrossValidationReports' progress instance, so that in total there is
only one progress instance.

But before this change, the progress instance was sometimes owned by a
`CrossValidationReport.metrics` accessor. This is a problem because
accessors are re-instantiated whenever they are accessed, so their state
cannot be modified from the parent.

The solution this change implements is to remove all `progress`-related
attributes from all accessors, and to ensure that the progress instance
is only owned by the Report object, not by any of its accessors.

Author: auguste-probabl

skore/src/skore/sklearn/_comparison/metrics_accessor.py

@@ -4,7 +4,6 @@
 import numpy as np
 import pandas as pd
 from numpy.typing import ArrayLike
-from rich.progress import Progress
 from sklearn.metrics import make_scorer
 from sklearn.metrics._scorer import _BaseScorer as SKLearnScorer
 from sklearn.utils.metaestimators import available_if
@@ -48,9 +47,6 @@ class _MetricsAccessor(_BaseAccessor, DirNamesMixin):
     def __init__(self, parent: ComparisonReport) -> None:
         super().__init__(parent)
 
-        self._progress_info: Optional[dict[str, Any]] = None
-        self._parent_progress: Optional[Progress] = None
-
     def report_metrics(
         self,
         *,
@@ -198,9 +194,9 @@ def _compute_metric_scores(
 
         cache_key = tuple(cache_key_parts)
 
-        assert self._progress_info is not None, "Progress info not set"
-        progress = self._progress_info["current_progress"]
-        main_task = self._progress_info["current_task"]
+        assert self._parent._progress_info is not None, "Progress info not set"
+        progress = self._parent._progress_info["current_progress"]
+        main_task = self._parent._progress_info["current_task"]
 
         total_estimators = len(self._parent.estimator_reports_)
         progress.update(main_task, total=total_estimators)
@@ -1272,9 +1268,9 @@ def _get_display(
             cache_key_parts.append(data_source)
             cache_key = tuple(cache_key_parts)
 
-        assert self._progress_info is not None, "Progress info not set"
-        progress = self._progress_info["current_progress"]
-        main_task = self._progress_info["current_task"]
+        assert self._parent._progress_info is not None, "Progress info not set"
+        progress = self._parent._progress_info["current_progress"]
+        main_task = self._parent._progress_info["current_task"]
         total_estimators = len(self._parent.estimator_reports_)
         progress.update(main_task, total=total_estimators)
 

skore/src/skore/sklearn/_cross_validation/metrics_accessor.py

@@ -3,7 +3,6 @@
 import joblib
 import numpy as np
 import pandas as pd
-from rich.progress import Progress
 from sklearn.metrics import make_scorer
 from sklearn.metrics._scorer import _BaseScorer as SKLearnScorer
 from sklearn.utils.metaestimators import available_if
@@ -48,9 +47,6 @@ class _MetricsAccessor(_BaseAccessor["CrossValidationReport"], DirNamesMixin):
     def __init__(self, parent: CrossValidationReport) -> None:
         super().__init__(parent)
 
-        self._progress_info: Optional[dict[str, Any]] = None
-        self._parent_progress: Optional[Progress] = None
-
     def report_metrics(
         self,
         *,
@@ -173,9 +169,9 @@ def _compute_metric_scores(
                 cache_key_parts.append(metric_kwargs[key])
         cache_key = tuple(cache_key_parts)
 
-        assert self._progress_info is not None, "Progress info not set"
-        progress = self._progress_info["current_progress"]
-        main_task = self._progress_info["current_task"]
+        assert self._parent._progress_info is not None, "Progress info not set"
+        progress = self._parent._progress_info["current_progress"]
+        main_task = self._parent._progress_info["current_task"]
 
         total_estimators = len(self._parent.estimator_reports_)
         progress.update(main_task, total=total_estimators)
@@ -979,9 +975,9 @@ def _get_display(
             cache_key_parts.append(data_source)
             cache_key = tuple(cache_key_parts)
 
-        assert self._progress_info is not None, "Progress info not set"
-        progress = self._progress_info["current_progress"]
-        main_task = self._progress_info["current_task"]
+        assert self._parent._progress_info is not None, "Progress info not set"
+        progress = self._parent._progress_info["current_progress"]
+        main_task = self._parent._progress_info["current_task"]
         total_estimators = len(self._parent.estimator_reports_)
         progress.update(main_task, total=total_estimators)
 

skore/src/skore/utils/_progress_bar.py

@@ -39,6 +39,10 @@ def decorator(func: Callable[..., T]) -> Callable[..., T]:
         def wrapper(*args: Any, **kwargs: Any) -> T:
             self_obj: Any = args[0]
 
+            if hasattr(self_obj, "_parent"):
+                # self_obj is an accessor
+                self_obj = self_obj._parent
+
             desc = description(self_obj) if callable(description) else description
 
             if getattr(self_obj, "_parent_progress", None) is not None: