Skip to content

Commit d35228d

Browse files
waridroxglemaitre
andauthored
fix: Show consistent favorability (↘︎) for Brier score with non-probabilistic classifiers (#1758)
Co-authored-by: Guillaume Lemaitre <[email protected]>
1 parent 8d08dac commit d35228d

File tree

2 files changed

+52
-4
lines changed

2 files changed

+52
-4
lines changed

skore/src/skore/sklearn/_comparison/utils.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,10 @@ def _combine_estimator_results(
7676
# - not use it in the aggregate operation
7777
# - later to only report a single column and not by split columns
7878
if indicator_favorability:
79-
favorability = results.pop("Favorability").iloc[:, 0]
79+
# Some metrics can be undefined for some estimators and NaN are
80+
# introduced after the concatenation. We fill the NaN using the
81+
# valid favorability
82+
favorability = results.pop("Favorability").bfill(axis=1).iloc[:, 0]
8083
else:
8184
favorability = None
8285

@@ -301,9 +304,14 @@ def sort_by_split(df: pd.DataFrame) -> pd.DataFrame:
301304
# - not use it in the aggregate operation
302305
# - later to only report a single column and not by split columns
303306
if indicator_favorability:
304-
favorability = results[0]["Favorability"]
305-
for result in results:
306-
result.pop("Favorability")
307+
# Some metrics can be undefined for some estimators and NaN are
308+
# introduced after the concatenation. We fill the NaN using the
309+
# valid favorability
310+
favorability = (
311+
pd.concat([result.pop("Favorability") for result in results], axis=1)
312+
.bfill(axis=1)
313+
.iloc[:, 0]
314+
)
307315
else:
308316
favorability = None
309317

skore/tests/unit/sklearn/comparison/test_report_common.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88

99
import joblib
1010
import pytest
11+
from sklearn.datasets import make_classification
12+
from sklearn.linear_model import LogisticRegression
13+
from sklearn.svm import LinearSVC
14+
from skore import ComparisonReport, CrossValidationReport, EstimatorReport
1115

1216

1317
@pytest.fixture(params=["report_estimator_reports", "report_cv_reports"])
@@ -67,3 +71,39 @@ def test_metrics_help(capsys, report):
6771
report.metrics.help()
6872
captured = capsys.readouterr()
6973
assert "Available metrics methods" in captured.out
74+
75+
76+
@pytest.mark.parametrize("report", [EstimatorReport, CrossValidationReport])
77+
def test_comparison_report_favorability_undefined_metrics(report):
78+
"""Check that we don't introduce NaN when favorability is computed when
79+
for some estimators, the metric is undefined.
80+
81+
Non-regression test for:
82+
https://github.com/probabl-ai/skore/issues/1755
83+
"""
84+
85+
X, y = make_classification(random_state=0)
86+
estimators = {"LinearSVC": LinearSVC(), "LogisticRegression": LogisticRegression()}
87+
88+
if report is EstimatorReport:
89+
reports = {
90+
name: EstimatorReport(est, X_train=X, X_test=X, y_train=y, y_test=y)
91+
for name, est in estimators.items()
92+
}
93+
else:
94+
reports = {
95+
name: CrossValidationReport(est, X=X, y=y)
96+
for name, est in estimators.items()
97+
}
98+
99+
comparison_report = ComparisonReport(reports)
100+
metrics = comparison_report.metrics.report_metrics(
101+
pos_label=1, indicator_favorability=True
102+
)
103+
104+
assert "Brier score" in metrics.index
105+
assert "Favorability" in metrics.columns
106+
assert not metrics["Favorability"].isna().any()
107+
expected_values = {"(↗︎)", "(↘︎)"}
108+
actual_values = set(metrics["Favorability"].to_numpy())
109+
assert actual_values.issubset(expected_values)

0 commit comments

Comments
 (0)