chore(skore/estimator-report): Clean some parts of the metrics accessor by auguste-probabl · Pull Request #2331 · probabl-ai/skore

auguste-probabl · 2026-01-23T17:24:28Z

In preparation for a larger refactor of _MetricsAccessor

thomass-dev · 2026-01-23T17:26:54Z

Please note that #2277 will result in significant changes to the metrics accessor.

github-actions · 2026-01-23T17:31:05Z

Documentation preview @ 43d7163

github-actions · 2026-01-23T17:35:00Z

Coverage Report for skore/

File	Stmts	Miss	Cover	Missing
skore/src/skore
__init__.py	25	0	100%
_config.py	31	0	100%
_login.py	14	0	100%
exceptions.py	4	4	0%	4, 15, 19, 23
skore/src/skore/_sklearn
__init__.py	6	0	100%
_base.py	199	14	92%	46, 59, 128, 131, 184, 187–188, 190–193, 226, 229–230
feature_names.py	26	0	100%
find_ml_task.py	61	0	100%
types.py	28	1	96%	30
skore/src/skore/_sklearn/_comparison
__init__.py	7	0	100%
inspection_accessor.py	32	1	96%	131
metrics_accessor.py	188	2	98%	178, 258
report.py	120	0	100%
utils.py	57	0	100%
skore/src/skore/_sklearn/_cross_validation
__init__.py	9	0	100%
data_accessor.py	45	2	95%	137, 140
inspection_accessor.py	21	0	100%
metrics_accessor.py	190	1	99%	252
report.py	140	1	99%	525
skore/src/skore/_sklearn/_estimator
__init__.py	9	0	100%
data_accessor.py	66	1	98%	82
inspection_accessor.py	83	2	97%	326, 340
metrics_accessor.py	370	5	98%	427, 431, 446, 476, 1749
report.py	167	2	98%	450–451
skore/src/skore/_sklearn/_plot
__init__.py	3	0	100%
base.py	107	6	94%	63–64, 266–268, 272
utils.py	121	2	98%	273–274
skore/src/skore/_sklearn/_plot/data
__init__.py	2	0	100%
table_report.py	175	1	99%	657
skore/src/skore/_sklearn/_plot/inspection
__init__.py	0	0	100%
coefficients.py	200	0	100%
permutation_importance.py	149	0	100%
utils.py	9	0	100%
skore/src/skore/_sklearn/_plot/metrics
__init__.py	6	0	100%
confusion_matrix.py	156	1	99%	546
metrics_summary_display.py	8	0	100%
precision_recall_curve.py	108	0	100%
prediction_error.py	151	0	100%
roc_curve.py	113	0	100%
skore/src/skore/_sklearn/train_test_split
__init__.py	0	0	100%
train_test_split.py	58	0	100%
skore/src/skore/_sklearn/train_test_split/warning
__init__.py	8	0	100%
high_class_imbalance_too_few_examples_warning.py	19	1	94%	83
high_class_imbalance_warning.py	20	0	100%
random_state_unset_warning.py	10	0	100%
shuffle_true_warning.py	9	0	100%
stratify_is_set_warning.py	10	0	100%
time_based_column_warning.py	21	0	100%
train_test_split_warning.py	3	0	100%
skore/src/skore/_utils
__init__.py	6	2	66%	8, 13
_accessor.py	107	3	97%	38, 207, 251
_cache.py	37	0	100%
_environment.py	27	0	100%
_fixes.py	8	0	100%
_index.py	5	0	100%
_logger.py	22	4	81%	15–17, 19
_measure_time.py	10	0	100%
_parallel.py	38	3	92%	23, 33, 124
_patch.py	21	12	42%	30, 35–39, 42–43, 46–47, 58, 60
_progress_bar.py	46	0	100%
_repr_html.py	8	0	100%
_show_versions.py	38	0	100%
_testing.py	60	1	98%	102
skore/src/skore/project
__init__.py	2	0	100%
_summary.py	75	1	98%	119
_widget.py	187	0	100%
project.py	56	0	100%
TOTAL	4117	73	98%

Tests	Skipped	Failures	Errors	Time
1475	5 💤	0 ❌	0 🔥	10m 15s ⏱️

thomass-dev

IMO, this PR looks like a PR that changes only the "styling code". This should be avoided when possible, or included in other changes when really needed.

Otherwise, we could do the same thing for all parts of the code...

thomass-dev · 2026-01-27T08:09:39Z

skore/src/skore/_sklearn/_estimator/metrics_accessor.py

+            if isinstance(score, list) and len(score) == 1:
+                score = score[0]
+            if isinstance(score, list) and "classification" in self._parent._ml_task:
+                score = dict(
                    zip(self._parent._estimator.classes_.tolist(), score, strict=False)
                )


Please use if/elif when the condition are exclusive. Same for your change on line 554.

~~@GaetandeCast @thomass-dev I've simplified the conditional but~~ the conditions are not quite exclusive enough to go further. Please advise.

reverted as I misunderstood what array.item() does (I thought it gave you the element of a 1-element array, but it also converts numpy-typed numbers to native types).

@thomass-dev please give it another look

auguste-probabl · 2026-01-27T13:31:55Z

IMO, this PR looks like a PR that changes only the "styling code". This should be avoided when possible, or included in other changes when really needed.

I did these changes while working on a larger refactor, and I wanted to make it as simple to review and merge as possible

GaetandeCast

I'm not against cleaning up the code even if it does not impact functionality. There is just one change I'm not convinced by.

skore/src/skore/_sklearn/_estimator/metrics_accessor.py

GaetandeCast · 2026-02-02T10:58:18Z

So I'm ok with this, although I agree with @thomass-dev 's remark about exclusive if/if statements.

skore/src/skore/_sklearn/_estimator/metrics_accessor.py

thomass-dev

LGTM now, thanks.

…or (probabl-ai#2331) In preparation for a larger refactor of `_MetricsAccessor`

thomass-dev requested changes Jan 27, 2026

View reviewed changes

thomass-dev changed the title ~~refactor: Various refactorings~~ chore(skore/estimator-report): Clean some parts of the metrics accessor Jan 27, 2026

GaetandeCast reviewed Jan 30, 2026

View reviewed changes

skore/src/skore/_sklearn/_estimator/metrics_accessor.py Show resolved Hide resolved

auguste-probabl added 6 commits February 5, 2026 12:14

Do extra manipulation before caching

9bdc9e7

Move metric-related logic together

8863920

inline variable

6070d6d

simplify ml task check

dc1888d

not sure about this one

74c0470

simplify conditionals

1f1c8fa

auguste-probabl force-pushed the push-ulwvtwwypxsp branch from df8b4e2 to 1f1c8fa Compare February 5, 2026 11:16

auguste-probabl added 2 commits February 5, 2026 12:40

revert

feed9ed

really revert lol

99d78ba

GaetandeCast reviewed Feb 5, 2026

View reviewed changes

skore/src/skore/_sklearn/_estimator/metrics_accessor.py Outdated Show resolved Hide resolved

nest logic more

43d7163

auguste-probabl requested a review from thomass-dev February 5, 2026 13:59

thomass-dev approved these changes Feb 6, 2026

View reviewed changes

thomass-dev enabled auto-merge February 6, 2026 14:48

thomass-dev disabled auto-merge February 6, 2026 14:48

thomass-dev merged commit bf47400 into probabl-ai:main Feb 6, 2026
35 checks passed

Sharkyii pushed a commit to Sharkyii/skore that referenced this pull request Feb 9, 2026

chore(skore/estimator-report): Clean some parts of the metrics access…

34fcaaa

…or (probabl-ai#2331) In preparation for a larger refactor of `_MetricsAccessor`

Sharkyii pushed a commit to Sharkyii/skore that referenced this pull request Feb 11, 2026

chore(skore/estimator-report): Clean some parts of the metrics access…

7f8ab82

…or (probabl-ai#2331) In preparation for a larger refactor of `_MetricsAccessor`

Sharkyii pushed a commit to Sharkyii/skore that referenced this pull request Feb 12, 2026

chore(skore/estimator-report): Clean some parts of the metrics access…

128859c

…or (probabl-ai#2331) In preparation for a larger refactor of `_MetricsAccessor`

Sharkyii pushed a commit to Sharkyii/skore that referenced this pull request Feb 12, 2026

chore(skore/estimator-report): Clean some parts of the metrics access…

7399f47

…or (probabl-ai#2331) In preparation for a larger refactor of `_MetricsAccessor`

Conversation

auguste-probabl commented Jan 23, 2026

Uh oh!

thomass-dev commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomass-dev left a comment

Choose a reason for hiding this comment

Uh oh!

thomass-dev Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

auguste-probabl Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

auguste-probabl Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

auguste-probabl Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

auguste-probabl commented Jan 27, 2026

Uh oh!

GaetandeCast left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GaetandeCast commented Feb 2, 2026

Uh oh!

Uh oh!

thomass-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thomass-dev commented Jan 23, 2026 •

edited

Loading

github-actions bot commented Jan 23, 2026 •

edited

Loading

github-actions bot commented Jan 23, 2026 •

edited

Loading

auguste-probabl Feb 5, 2026 •

edited

Loading

GaetandeCast left a comment •

edited

Loading