Skip to content

refactor(plot): return plotting objects instead of storing attributes#2396

Open
Sharkyii wants to merge 6 commits intoprobabl-ai:mainfrom
Sharkyii:fix-display-plot
Open

refactor(plot): return plotting objects instead of storing attributes#2396
Sharkyii wants to merge 6 commits intoprobabl-ai:mainfrom
Sharkyii:fix-display-plot

Conversation

@Sharkyii
Copy link
Contributor

@Sharkyii Sharkyii commented Feb 9, 2026

This PR refactors all Display.plot() methods to return the plotting object instead of storing them as attributes on the display instance.

Previously, calling .plot() created attributes such as figure_, ax_,. These attributes did not exist before plotting, which introduced hidden state and allowed external mutation of internal display objects.

Fixes #2374

Follow up -
Edit other test file to work with the new pattern

@Sharkyii Sharkyii marked this pull request as draft February 9, 2026 20:45
@Sharkyii Sharkyii force-pushed the fix-display-plot branch 3 times, most recently from 61b2cfb to 7f864d3 Compare February 10, 2026 15:34
@Sharkyii
Copy link
Contributor Author

@auguste-probabl could you review this PR?
If the direction looks good, I’ll continue updating the remaining tests.

@auguste-probabl
Copy link
Collaborator

If the direction looks good, I’ll continue updating the remaining tests.

Yep, I think this works fine, thanks!

@Sharkyii Sharkyii force-pushed the fix-display-plot branch 2 times, most recently from 5b46536 to 87fd814 Compare February 11, 2026 21:12
@Sharkyii
Copy link
Contributor Author

@auguste-probabl could you please review this when you have a moment? I’ll double-check again in the meantime to ensure all tests are passing.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2026

Documentation preview @ 7909769

@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2026

Coverage

Coverage Report for skore/
FileStmtsMissCoverMissing
skore/src/skore
   __init__.py260100% 
   _config.py310100% 
   _login.py140100% 
   exceptions.py440%4, 15, 19, 23
skore/src/skore/_sklearn
   __init__.py60100% 
   _base.py73889%272–279
   feature_names.py260100% 
   find_ml_task.py610100% 
   types.py28196%30
skore/src/skore/_sklearn/_comparison
   __init__.py70100% 
   inspection_accessor.py27196%124
   metrics_accessor.py166298%179, 1138
   report.py111397%488, 491, 497
   utils.py570100% 
skore/src/skore/_sklearn/_cross_validation
   __init__.py90100% 
   data_accessor.py400100% 
   inspection_accessor.py20195%116
   metrics_accessor.py169597%1054, 1115, 1118, 1144–1145
   report.py126397%498, 501, 507
skore/src/skore/_sklearn/_estimator
   __init__.py90100% 
   data_accessor.py63296%86, 190
   inspection_accessor.py70297%315, 329
   metrics_accessor.py367698%427, 431, 446, 476, 1661, 1744
   report.py158298%463, 466
skore/src/skore/_sklearn/_plot
   __init__.py30100% 
   base.py57296%59–60
   utils.py121298%273–274
skore/src/skore/_sklearn/_plot/data
   __init__.py20100% 
   table_report.py178199%669
skore/src/skore/_sklearn/_plot/inspection
   __init__.py00100% 
   coefficients.py2100100% 
   impurity_decrease.py62198%175
   permutation_importance.py1520100% 
   utils.py90100% 
skore/src/skore/_sklearn/_plot/metrics
   __init__.py60100% 
   confusion_matrix.py1570100% 
   metrics_summary_display.py80100% 
   precision_recall_curve.py1090100% 
   prediction_error.py1520100% 
   roc_curve.py1130100% 
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.py112397%38, 214, 268
   _cache.py370100% 
   _environment.py32293%41, 44
   _fixes.py80100% 
   _index.py50100% 
   _logger.py22481%15–17, 19
   _measure_time.py100100% 
   _parallel.py38392%23, 33, 124
   _patch.py211242%30, 35–39, 42–43, 46–47, 58, 60
   _progress_bar.py41490%55–56, 66–67
   _show_versions.py380100% 
   _testing.py1031189%20, 29, 157, 166, 177–182, 184
skore/src/skore/_utils/repr
   __init__.py20100% 
   base.py540100% 
   data.py1630100% 
   html_repr.py380100% 
   rich_repr.py810100% 
skore/src/skore/project
   __init__.py20100% 
   _summary.py75198%119
   _widget.py1870100% 
   project.py550100% 
TOTAL42958997% 

Tests Skipped Failures Errors Time
1649 5 💤 0 ❌ 0 🔥 6m 8s ⏱️

@Sharkyii Sharkyii changed the title Return plotting objects instead of storing attributes refactor(plot): return plotting objects instead of storing attributes Feb 12, 2026
@Sharkyii Sharkyii force-pushed the fix-display-plot branch 2 times, most recently from 81aa88b to c167f88 Compare February 12, 2026 16:28
@Sharkyii
Copy link
Contributor Author

@auguste-probabl All tests are passing on my side - ready for review.

@Sharkyii Sharkyii marked this pull request as ready for review February 12, 2026 16:37
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.

api(skore): Change the return values of .plot() and the attribute of Display

2 participants