Use env variable to configure matplotlib backend #8113
Conversation
LukasMasuch
changed the title
…ar-for-matplotlib-fix
…ar-for-matplotlib-fix
…ar-for-matplotlib-fix
LukasMasuch
changed the title
LukasMasuch
changed the title
There was a problem hiding this comment.
LGTM -- is there any possibility that doing this vs environment var will result in us changing the matplotlib backend for users in places they wouldn't have expected it?
I'd expect this is pretty close to being a total non-issue in practice, but just double-checking.
lib/streamlit/__init__.py
Outdated
| # Set Matplotlib backend to avoid a crash. | ||
| # The default Matplotlib backend crashes Python on OSX when run on a thread | ||
| # that's not the main thread, so here we set a safer backend as a fix. | ||
| # This fix is OS-independent. We didn't see a good reason to make this | ||
| # Mac-only. Consistency within Streamlit seemed more important. | ||
| # This needs to run before any other import of matplotlib could happen. | ||
| import os as _os | ||
|
|
||
| _os.environ["MPLBACKEND"] = "Agg" |
There was a problem hiding this comment.
nit: it'd probably be good to mention in the comment that this needs to stay where it is before any other imports to ensure that it occurs before matplotlib has a chance to be imported
There was a problem hiding this comment.
See the last line of comment above the os :)
This needs to run before any other import of matplotlib could happen.
I will copy this moment down above the environ and add an Important to make this more prominent.
There is definitely a risk. I think I'm ~80% confident it shouldn't have any negative side effects. I will be extra careful to monitor the forum and Github issues for anything Matplotlib-related. And will get this reverted via a patch if anything concerning comes up. |
…ar-for-matplotlib-fix
## Describe your changes The deprecation of the `runner.fixMatplotlib` and the decision to always use the `Agg` backend, made it possible to just configure the matplotlib backend via the config option (also see the previous TODO comment). This prevents an unnecessary import of matplotlib at the server start and allows to lazy load this import. ## GitHub Issue Link (if applicable) Related to streamlit#6066 ## Testing Plan - Added unit and e2e tests to make sure that `matplotlib` is properly lazy-loaded. --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
Describe your changes
The deprecation of the
runner.fixMatplotliband the decision to always use theAggbackend, made it possible to just configure the matplotlib backend via the config option (also see the previous TODO comment). This prevents an unnecessary import of matplotlib at the server start and allows to lazy load this import.GitHub Issue Link (if applicable)
Related to #6066
Testing Plan
matplotlibis properly lazy-loaded.Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.