Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(OSX): Matplotlib backend leaks in App Switcher #1232

Merged
merged 4 commits into from
Jan 30, 2025

Conversation

rouk1
Copy link
Contributor

@rouk1 rouk1 commented Jan 27, 2025

When the figureproperty of MatplotlibFigureItemis invoked, joblib reloads the plot and instantiate the figure.
If the backend is OSX and the figure is not released then a headless window is created and never dies.
Screenshot 2025-01-27 at 14 43 24

This PR introduces a context switcher that forces the property to be called with the agg backend.

Copy link
Contributor

github-actions bot commented Jan 27, 2025

Coverage

Coverage Report for backend
FileStmtsMissCoverMissing
venv/lib/python3.12/site-packages/skore
   __init__.py140100% 
   __main__.py880%3–19
   exceptions.py330%4–19
venv/lib/python3.12/site-packages/skore/cli
   __init__.py550%3–8
   cli.py16160%3–59
   color_format.py49490%3–116
venv/lib/python3.12/site-packages/skore/persistence
   __init__.py00100% 
venv/lib/python3.12/site-packages/skore/persistence/item
   __init__.py59491%89, 100–103
   altair_chart_item.py24193%15
   cross_validation_reporter_item.py1091289%28–41, 125–126, 259, 262
   item.py24196%86
   matplotlib_figure_item.py42196%19
   media_item.py240100% 
   numpy_array_item.py29194%16
   pandas_dataframe_item.py31194%14
   pandas_series_item.py31194%14
   pickle_item.py330100% 
   pillow_image_item.py32194%16
   plotly_figure_item.py25193%15
   polars_dataframe_item.py29194%14
   polars_series_item.py24193%14
   primitive_item.py25291%13–15
   sklearn_base_estimator_item.py31194%15
   skrub_table_report_item.py10186%11
venv/lib/python3.12/site-packages/skore/persistence/repository
   __init__.py30100% 
   item_repository.py59591%15–16, 202–203, 226
   view_repository.py19286%9–10
venv/lib/python3.12/site-packages/skore/persistence/storage
   __init__.py40100% 
   abstract_storage.py220100% 
   disk_cache_storage.py33195%44
   in_memory_storage.py200100% 
venv/lib/python3.12/site-packages/skore/persistence/view
   __init__.py20100% 
   view.py50100% 
venv/lib/python3.12/site-packages/skore/project
   __init__.py20100% 
   project.py62199%240
venv/lib/python3.12/site-packages/skore/sklearn
   __init__.py50100% 
   _base.py140497%91, 94, 168–>173, 183–184
   find_ml_task.py45195%71–>87, 80–>87, 86
   types.py20100% 
venv/lib/python3.12/site-packages/skore/sklearn/_cross_validation
   __init__.py60100% 
   metrics_accessor.py1630100% 
   report.py870100% 
venv/lib/python3.12/site-packages/skore/sklearn/_estimator
   __init__.py60100% 
   metrics_accessor.py265497%149–158, 183–>236, 191, 434–>437, 783–>786
   report.py120099%213–>219, 221–>223
   utils.py11110%1–19
venv/lib/python3.12/site-packages/skore/sklearn/_plot
   __init__.py40100% 
   precision_recall_curve.py121198%229–>246, 317
   prediction_error.py970100% 
   roc_curve.py1280100% 
   utils.py880100% 
venv/lib/python3.12/site-packages/skore/sklearn/cross_validation
   __init__.py20100% 
   cross_validation_helpers.py47490%104–>136, 123–126
   cross_validation_reporter.py35195%187
venv/lib/python3.12/site-packages/skore/sklearn/cross_validation/plots
   __init__.py00100% 
   compare_scores_plot.py292416%10, 28–116
   timing_plot.py292417%10, 26–123
venv/lib/python3.12/site-packages/skore/sklearn/train_test_split
   __init__.py00100% 
   train_test_split.py36294%16–17
venv/lib/python3.12/site-packages/skore/sklearn/train_test_split/warning
   __init__.py80100% 
   high_class_imbalance_too_few_examples_warning.py17378%16–18, 80
   high_class_imbalance_warning.py18288%16–18
   random_state_unset_warning.py11187%15
   shuffle_true_warning.py9091%44–>exit
   stratify_is_set_warning.py11187%15
   time_based_column_warning.py22189%17, 69–>exit
   train_test_split_warning.py5180%21
venv/lib/python3.12/site-packages/skore/ui
   __init__.py00100% 
   app.py32868%31, 60–65, 77–79
   dependencies.py40100% 
   launch.py261539%36–57
   project_routes.py620100% 
venv/lib/python3.12/site-packages/skore/utils
   __init__.py60100% 
   _accessor.py70100% 
   _logger.py21484%14–18
   _patch.py11546%19–35
   _progress_bar.py300100% 
   _show_versions.py310100% 
TOTAL270523791% 

Tests Skipped Failures Errors Time
589 3 💤 0 ❌ 0 🔥 3m 53s ⏱️

Copy link
Contributor

github-actions bot commented Jan 27, 2025

Documentation preview @ eafdab2

@glemaitre
Copy link
Member

And if it happen not in the main thread, then it makes a segfault ;)

@rouk1 rouk1 force-pushed the matplotlib-backend branch from 14d5de7 to c4c0a5d Compare January 28, 2025 08:42
@auguste-probabl auguste-probabl changed the title fix (OSX): Matplotlib backend leaks in APP Switcher fix(OSX): Matplotlib backend leaks in App Switcher Jan 28, 2025
glemaitre
glemaitre previously approved these changes Jan 29, 2025
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @rouk1

thomass-dev
thomass-dev previously approved these changes Jan 30, 2025
Copy link
Collaborator

@thomass-dev thomass-dev left a comment

Choose a reason for hiding this comment

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

Well spotted @rouk1 , thanks!

@glemaitre
Copy link
Member

Whoops some conflicts ;)

@rouk1 rouk1 dismissed stale reviews from thomass-dev and glemaitre via eafdab2 January 30, 2025 10:47
@rouk1
Copy link
Contributor Author

rouk1 commented Jan 30, 2025

Conflicts have been resolved.

@glemaitre glemaitre merged commit 4f72706 into probabl-ai:main Jan 30, 2025
18 checks passed
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