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

Use env variable to configure matplotlib backend #8113

Merged
merged 18 commits into from Feb 10, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion e2e_playwright/lazy_loaded_modules.py
Expand Up @@ -24,6 +24,7 @@
"altair",
"graphviz",
"watchdog",
"matplotlib",
"pandas",
"pyarrow",
"streamlit.emojis",
Expand All @@ -33,7 +34,6 @@
# TODO(lukasmasuch): Lazy load more packages:
# "streamlit.hello",
# "numpy",
# "matplotlib",
# "plotly",
# "pillow",
]
Expand Down
2 changes: 1 addition & 1 deletion e2e_playwright/lazy_loaded_modules_test.py
Expand Up @@ -20,6 +20,6 @@
def test_lazy_loaded_modules_are_not_imported(app: Page):
"""Test that lazy loaded modules are not imported when the page is loaded."""
markdown_elements = app.get_by_test_id("stMarkdown")
expect(markdown_elements).to_have_count(13)
expect(markdown_elements).to_have_count(14)
for element in markdown_elements.all():
expect(element).to_have_text(re.compile(r".*not loaded.*"))
12 changes: 12 additions & 0 deletions lib/streamlit/__init__.py
Expand Up @@ -44,6 +44,18 @@

# IMPORTANT: Prefix with an underscore anything that the user shouldn't see.


# 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"
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.



# Must be at the top, to avoid circular dependency.
from streamlit import logger as _logger
from streamlit import config as _config
Expand Down
3 changes: 0 additions & 3 deletions lib/streamlit/testing/v1/app_test.py
Expand Up @@ -82,12 +82,9 @@
)
from streamlit.testing.v1.local_script_runner import LocalScriptRunner
from streamlit.util import HASHLIB_KWARGS
from streamlit.web.bootstrap import _fix_matplotlib_crash

TMP_DIR = tempfile.TemporaryDirectory()

_fix_matplotlib_crash()


class AppTest:
"""
Expand Down
43 changes: 8 additions & 35 deletions lib/streamlit/web/bootstrap.py
Expand Up @@ -12,13 +12,16 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import asyncio
import os
import signal
import sys
from pathlib import Path
from typing import Any, Dict, List
from typing import Any, Dict, Final, List

# TODO(lukasmasuch): Lazy-load this module:
import click

from streamlit import (
Expand All @@ -38,9 +41,9 @@
from streamlit.watcher import report_watchdog_availability, watch_dir, watch_file
from streamlit.web.server import Server, server_address_is_unix_socket, server_util

LOGGER = get_logger(__name__)
_LOGGER: Final = get_logger(__name__)

NEW_VERSION_TEXT = """
NEW_VERSION_TEXT: Final = """
%(new_version)s

See what's new at https://discuss.streamlit.io/c/announcements
Expand All @@ -62,7 +65,7 @@


def _set_up_signal_handler(server: Server) -> None:
LOGGER.debug("Setting up signal handler")
_LOGGER.debug("Setting up signal handler")

def signal_handler(signal_number, stack_frame):
# The server will shut down its threads and exit its loop.
Expand All @@ -85,35 +88,6 @@ def _fix_sys_path(main_script_path: str) -> None:
sys.path.insert(0, os.path.dirname(main_script_path))


def _fix_matplotlib_crash() -> None:
"""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.
Users can always disable this behavior by setting the config
runner.fixMatplotlib = false.

This fix is OS-independent. We didn't see a good reason to make this
Mac-only. Consistency within Streamlit seemed more important.
"""
if config.get_option("runner.fixMatplotlib"):
try:
# TODO: a better option may be to set
# os.environ["MPLBACKEND"] = "Agg". We'd need to do this towards
# the top of __init__.py, before importing anything that imports
# pandas (which imports matplotlib). Alternately, we could set
# this environment variable in a new entrypoint defined in
# setup.py. Both of these introduce additional trickiness: they
# need to run without consulting streamlit.config.get_option,
# because this would import streamlit, and therefore matplotlib.
import matplotlib

matplotlib.use("Agg")
except ImportError:
# Matplotlib is not installed. No need to do anything.
pass


def _fix_tornado_crash() -> None:
"""Set default asyncio policy to be compatible with Tornado 6.

Expand Down Expand Up @@ -168,7 +142,7 @@ def _on_server_start(server: Server) -> None:
try:
secrets.load_if_toml_exists()
except Exception as ex:
LOGGER.error(f"Failed to load secrets.toml file", exc_info=ex)
_LOGGER.error(f"Failed to load secrets.toml file", exc_info=ex)

def maybe_open_browser():
if config.get_option("server.headless"):
Expand Down Expand Up @@ -402,7 +376,6 @@ def run(
This starts a blocking asyncio eventloop.
"""
_fix_sys_path(main_script_path)
_fix_matplotlib_crash()
_fix_tornado_crash()
_fix_sys_argv(main_script_path, args)
_fix_pydeck_mapbox_api_warning()
Expand Down
21 changes: 21 additions & 0 deletions lib/tests/streamlit/streamlit_test.py
Expand Up @@ -21,6 +21,8 @@
import tempfile
import unittest

import matplotlib

import streamlit as st
from streamlit import __version__

Expand Down Expand Up @@ -48,6 +50,25 @@ def test_get_option(self):
# This is set in lib/tests/conftest.py to False
self.assertEqual(False, st.get_option("browser.gatherUsageStats"))

def test_matplotlib_uses_agg(self):
"""Test that Streamlit uses the 'Agg' backend for matplotlib."""
ORIG_PLATFORM = sys.platform

for platform in ["darwin", "linux2"]:
sys.platform = platform

self.assertEqual(matplotlib.get_backend().lower(), "agg")
self.assertEqual(os.environ.get("MPLBACKEND").lower(), "agg")

# Force matplotlib to use a different backend
matplotlib.use("pdf", force=True)
self.assertEqual(matplotlib.get_backend().lower(), "pdf")

# Reset the backend to 'Agg'
matplotlib.use("agg", force=True)
self.assertEqual(matplotlib.get_backend().lower(), "agg")
sys.platform = ORIG_PLATFORM

def test_public_api(self):
"""Test that we don't accidentally remove (or add) symbols
to the public `st` API.
Expand Down
24 changes: 1 addition & 23 deletions lib/tests/streamlit/web/bootstrap_test.py
Expand Up @@ -18,7 +18,6 @@
from unittest import IsolatedAsyncioTestCase
from unittest.mock import Mock, patch

import matplotlib
import pytest

from streamlit import config
Expand All @@ -31,27 +30,6 @@
from tests.testutil import patch_config_options, should_skip_pydantic_tests


class BootstrapTest(unittest.TestCase):
@patch("streamlit.web.bootstrap.asyncio.run", Mock())
@patch("streamlit.web.bootstrap.Server", Mock())
@patch("streamlit.web.bootstrap._install_pages_watcher", Mock())
def test_fix_matplotlib_crash(self):
"""Test that bootstrap.run sets the matplotlib backend to
"Agg".
"""
# TODO: Find a proper way to mock sys.platform
ORIG_PLATFORM = sys.platform

for platform in ["darwin", "linux2"]:
sys.platform = platform

matplotlib.use("pdf", force=True)
bootstrap.run("/not/a/script", "", [], {})
self.assertEqual("agg", matplotlib.get_backend().lower())

sys.platform = ORIG_PLATFORM


class BootstrapPydanticFixTest(unittest.TestCase):
def pydantic_model_definition(self):
from pydantic import BaseModel, root_validator, validator
Expand Down Expand Up @@ -451,7 +429,7 @@ def test_load_secrets(self, mock_load_secrets):

@patch("streamlit.web.bootstrap.asyncio.get_running_loop", Mock())
@patch("streamlit.web.bootstrap._maybe_print_static_folder_warning", Mock())
@patch("streamlit.web.bootstrap.LOGGER.error")
@patch("streamlit.web.bootstrap._LOGGER.error")
@patch("streamlit.web.bootstrap.secrets.load_if_toml_exists")
def test_log_secret_load_error(self, mock_load_secrets, mock_log_error):
"""If secrets throws an error on startup, we catch and log it."""
Expand Down