test: Add data_source parameter tests for ROC curve and ComparisonReport

[sobiya-22]

Closes #1747

Hi @glemaitre , @MarieSacksick
Added unit tests for:

• data_source parameter functionality in ROC curve
• ComparisonReport display verification

[glemaitre]

@sobiya-22 Thanks for the PR but it is not really what we would like as a test.

To give an example, in EstimatorReport, we test the behaviour of data_source with the following test:

https://github.com/probabl-ai/skore/blob/main/skore/tests/unit/sklearn/plot/roc_curve/test_estimator.py#L113-L158

Here, I would expect to modify the files skore/tests/unit/sklearn/plot/roc_curve/test_comparison_estimator.py to add similar test for the case that we have a ComparisonReport built with EstimatorReports to check that the data_source parameter has the expected behaviour.

We can limit the scope of this PR to this specific behaviour but I think that we need to extend the same test to the test_comparison_cross_validation.py file and most probably for the other type of visualization (PR curve and prediction-error).

[sobiya-22]

Hi @glemaitre ,
Thank you for the feedback! I've now implemented the data_source parameter tests for ROC curve visualization in test_comparison_estimator.py and test_comparison_cross_validation.py

skore/tests/unit/sklearn/plot/roc_curve/test_comparison_estimator.py - Added test_data_source_binary_classification() and test_data_source_multiclass_classification()
skore/tests/unit/sklearn/plot/roc_curve/test_comparison_cross_validation.py - Added corresponding tests for CrossValidationReport scenarios

Both test files now include comprehensive coverage of the data_source parameter behavior ("train", "test", and "X_y" options) following the same pattern as the EstimatorReport tests you referenced.

Remaining task:
PR curve visualization tests (precision_recall curve)
Prediction-error visualization tests

Before I proceed with implementing similar data_source tests for PR curve and prediction-error visualizations, could you please review the current ROC curve implementation to confirm I'm on the right track? I want to ensure the approach and test structure are correct before extending to the remaining visualization types.

Please let me know if this matches your expectations, and I'll proceed with the PR curve and prediction-error tests accordingly.
Thanks!

[glemaitre]

Before I proceed with implementing similar data_source tests for PR curve and prediction-error visualizations, could you please review the current ROC curve implementation to confirm I'm on the right track?

Indeed, it should be implemented in some independent PRs to keep the scope focused. I'll make a review.

[github-actions]

Coverage Report for skore/

File	Stmts	Miss	Cover	Missing
venv/lib/python3.13/site-packages/skore
__init__.py	23	0	100%
_config.py	28	0	100%
exceptions.py	4	4	0%	4, 15, 19, 23
venv/lib/python3.13/site-packages/skore/project
__init__.py	2	0	100%
project.py	49	0	100%
summary.py	74	0	100%
widget.py	138	5	96%	375–377, 447–448
venv/lib/python3.13/site-packages/skore/sklearn
__init__.py	7	0	100%
_base.py	169	14	91%	45, 58, 126, 129, 182, 185–186, 188–191, 224, 227–228
find_estimators.py	27	0	100%
find_ml_task.py	61	0	100%
types.py	26	1	96%	26
venv/lib/python3.13/site-packages/skore/sklearn/_comparison
__init__.py	5	0	100%
metrics_accessor.py	204	3	98%	175, 255, 1298
report.py	105	0	100%
utils.py	55	0	100%
venv/lib/python3.13/site-packages/skore/sklearn/_cross_validation
__init__.py	5	0	100%
metrics_accessor.py	209	1	99%	248
report.py	122	1	99%	466
venv/lib/python3.13/site-packages/skore/sklearn/_estimator
__init__.py	7	0	100%
feature_importance_accessor.py	143	2	98%	216–217
metrics_accessor.py	382	8	97%	195, 197, 204, 295, 364, 368, 383, 418
report.py	164	2	98%	438–439
venv/lib/python3.13/site-packages/skore/sklearn/_plot
__init__.py	2	0	100%
base.py	5	0	100%
style.py	28	0	100%
utils.py	118	5	95%	50, 74–76, 80
venv/lib/python3.13/site-packages/skore/sklearn/_plot/metrics
__init__.py	6	0	100%
confusion_matrix.py	69	4	94%	90, 98, 120, 228
precision_recall_curve.py	260	5	98%	459, 559, 563, 623, 743
prediction_error.py	215	5	97%	179, 186, 422, 505, 687
roc_curve.py	272	5	98%	386, 509, 617, 626, 819
summarize.py	7	0	100%
venv/lib/python3.13/site-packages/skore/sklearn/train_test_split
__init__.py	0	0	100%
train_test_split.py	49	0	100%
venv/lib/python3.13/site-packages/skore/sklearn/train_test_split/warning
__init__.py	8	0	100%
high_class_imbalance_too_few_examples_warning.py	17	1	94%	80
high_class_imbalance_warning.py	18	0	100%
random_state_unset_warning.py	10	0	100%
shuffle_true_warning.py	10	1	90%	46
stratify_is_set_warning.py	10	0	100%
time_based_column_warning.py	21	1	95%	73
train_test_split_warning.py	4	0	100%
venv/lib/python3.13/site-packages/skore/utils
__init__.py	6	2	66%	8, 13
_accessor.py	52	2	96%	67, 108
_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	13	5	61%	21, 23–24, 35, 37
_progress_bar.py	45	0	100%
_show_versions.py	33	2	93%	65–66
_testing.py	46	0	100%
TOTAL	3443	86	97%

Tests	Skipped	Failures	Errors	Time
873	5 💤	0 ❌	0 🔥	6m 58s ⏱️

[github-actions]

Documentation preview @ 7c08f39

[sobiya-22]

Hi @glemaitre,
I've now removed self-explanatory comments as suggested.
Please let me know if anything else needs to be addressed.
Once approved, I’ll proceed with the separate PRs for the PR curve and prediction-error visualizations as you recommended.

[sobiya-22]

Hi @glemaitre , Shall I proceed with separate PR's for PR curve and prediction-error visualizations?

[MarieSacksick]

Almost good to me :)!
To be sure that the data_source is really used in the plot, and not just always the train or the test, can you check that there are some changes, other than in the parameter data_source please?

[MarieSacksick]

Can you check that the results when changing the data_source do change please?
For instance, you can use a subsample of X and y when giving the data source X_y so you don't have to create a new one, and then check that the dataframes display.frame() outputed are different with assert not and the equals function.

[MarieSacksick]

Same as above, can you check that the outputs are different please?

[MarieSacksick]

same as in previous file: checking that the outputs are different.