Skip to content

Conversation

@glemaitre
Copy link
Member

@glemaitre glemaitre commented Sep 10, 2025

Partially fixing #1996

This PR implement a specialized plot method for each display. It allows to get the proper documentation that was broken when using a mixin that overwrite it.

This approach is simpler than trying to overwrite the name function, signature, and documentation of the dispatch plotting function. It should be easier to maintain.

While implementing this PR, I also fix the following:

  • group all the mixin in base.py and create a DisplayMixin since all specialized displays will inherit from all those mixins.
  • I fixed slightly the ConfusionMatrixDisplay but it will need more work.
  • I also quickly fixed the FeatureImportanceDisplay but it also need some more work.

@glemaitre glemaitre changed the title fix(skore): Document via explicit implementation fix(skore): Document plot via explicit implementation Sep 10, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 10, 2025

Coverage

Coverage Report for skore/
FileStmtsMissCoverMissing
skore/src/skore
   __init__.py230100% 
   _config.py310100% 
   exceptions.py440%4, 15, 19, 23
skore/src/skore/_sklearn
   __init__.py60100% 
   _base.py1981492%45, 58, 127, 130, 183, 186–187, 189–192, 225, 228–229
   find_ml_task.py610100% 
   types.py27196%28
skore/src/skore/_sklearn/_comparison
   __init__.py70100% 
   feature_importance_accessor.py39294%90, 109
   metrics_accessor.py178398%173, 253, 1215
   report.py1070100% 
   utils.py540100% 
skore/src/skore/_sklearn/_cross_validation
   __init__.py90100% 
   data_accessor.py45393%128, 131, 134
   feature_importance_accessor.py240100% 
   metrics_accessor.py182199%244
   report.py135199%487
skore/src/skore/_sklearn/_estimator
   __init__.py90100% 
   data_accessor.py580100% 
   feature_importance_accessor.py144298%223–224
   metrics_accessor.py356897%200, 202, 209, 300, 369, 373, 388, 423
   report.py167298%448–449
skore/src/skore/_sklearn/_plot
   __init__.py30100% 
   base.py102793%61–62, 201, 225–227, 231
   utils.py770100% 
skore/src/skore/_sklearn/_plot/data
   __init__.py20100% 
   table_report.py185199%706
skore/src/skore/_sklearn/_plot/metrics
   __init__.py60100% 
   confusion_matrix.py70494%92, 100, 122, 230
   feature_importance_display.py672168%88, 121–122, 124, 142–146, 148–155, 158–160, 162
   metrics_summary_display.py80100% 
   precision_recall_curve.py280598%455, 555, 559, 619, 751
   prediction_error.py227597%179, 186, 422, 505, 705
   roc_curve.py292897%385, 508, 513, 614, 619, 623, 692, 832
skore/src/skore/_sklearn/train_test_split
   __init__.py00100% 
   train_test_split.py580100% 
skore/src/skore/_sklearn/train_test_split/warning
   __init__.py80100% 
   high_class_imbalance_too_few_examples_warning.py19194%83
   high_class_imbalance_warning.py200100% 
   random_state_unset_warning.py100100% 
   shuffle_true_warning.py90100% 
   stratify_is_set_warning.py100100% 
   time_based_column_warning.py210100% 
   train_test_split_warning.py30100% 
skore/src/skore/_utils
   __init__.py6266%8, 13
   _accessor.py90396%34, 146, 190
   _environment.py270100% 
   _fixes.py80100% 
   _index.py50100% 
   _logger.py22481%15–17, 19
   _measure_time.py100100% 
   _parallel.py38392%23, 33, 124
   _patch.py13561%21, 23–24, 35, 37
   _progress_bar.py460100% 
   _repr_html.py80100% 
   _show_versions.py380100% 
   _testing.py550100% 
skore/src/skore/project
   __init__.py20100% 
   project.py480100% 
   summary.py740100% 
   widget.py165696%436, 439–441, 525–526
TOTAL399611697% 

Tests Skipped Failures Errors Time
1060 5 💤 0 ❌ 0 🔥 4m 21s ⏱️

@github-actions
Copy link
Contributor

github-actions bot commented Sep 10, 2025

Documentation preview @ 60306dc

This decorator:
1. Applies default style settings
2. Executes `plot_func`
3. Applies `tight_layout`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it apply tight layout?

@auguste-probabl
Copy link
Contributor

I can confirm that this solves the issue, using the reproducer given here

Copy link
Contributor

@auguste-probabl auguste-probabl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, awesome!

@thomass-dev thomass-dev merged commit 6d831a9 into probabl-ai:main Sep 11, 2025
32 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants