From f42ad67ac3fcd5930415751bce93c327f1e5db6f Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 26 Aug 2019 21:39:52 -0500 Subject: [PATCH 1/6] Validate plot backend when setting. ```python In [1]: import pandas as pd In [2]: import sys In [3]: import types In [4]: module = types.ModuleType("foo") In [5]: sys.modules['foo'] = module In [6]: pd.set_option('plotting.backend', 'foo') --------------------------------------------------------------------------- ValueError Traceback (most recent call last) in ----> 1 pd.set_option('plotting.backend', 'foo') ... ~/sandbox/pandas/pandas/plotting/_core.py in _find_backend(backend) 1588 "top-level `.plot` method." 1589 ) -> 1590 raise ValueError(msg.format(name=backend)) 1591 1592 ValueError: Could not find plotting backend 'foo'. Ensure that you've installed the package providing the 'foo' entrypoint, or that the package has atop-level `.plot` method. ``` Closes https://github.com/pandas-dev/pandas/issues/28163 --- doc/source/whatsnew/v1.0.0.rst | 1 + pandas/core/config_init.py | 24 +++--------------- pandas/plotting/_core.py | 24 ++++++++++++++---- pandas/tests/plotting/test_backend.py | 35 +++++++++++++++++++-------- 4 files changed, 48 insertions(+), 36 deletions(-) diff --git a/doc/source/whatsnew/v1.0.0.rst b/doc/source/whatsnew/v1.0.0.rst index 7fe358d3820f2..75f9317f8f10b 100644 --- a/doc/source/whatsnew/v1.0.0.rst +++ b/doc/source/whatsnew/v1.0.0.rst @@ -171,6 +171,7 @@ Plotting - Bug in :meth:`DataFrame.plot` producing incorrect legend markers when plotting multiple series on the same axis (:issue:`18222`) - Bug in :meth:`DataFrame.plot` when ``kind='box'`` and data contains datetime or timedelta data. These types are now automatically dropped (:issue:`22799`) - Bug in :meth:`DataFrame.plot.line` and :meth:`DataFrame.plot.area` produce wrong xlim in x-axis (:issue:`27686`, :issue:`25160`, :issue:`24784`) +- :func:`set_option` now validates that the plot backend provided to ``'plotting.backend'`` implements the backend when the option is set, rather than when a plot is created (:issue:`28163`) Groupby/resample/rolling ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/pandas/core/config_init.py b/pandas/core/config_init.py index 08dce6aca6e6d..276f5d8478beb 100644 --- a/pandas/core/config_init.py +++ b/pandas/core/config_init.py @@ -9,8 +9,6 @@ module is imported, register them here rather then in the module. """ -import importlib - import pandas._config.config as cf from pandas._config.config import ( is_bool, @@ -581,26 +579,10 @@ def use_inf_as_na_cb(key): def register_plotting_backend_cb(key): - backend_str = cf.get_option(key) - if backend_str == "matplotlib": - try: - import pandas.plotting._matplotlib # noqa - except ImportError: - raise ImportError( - "matplotlib is required for plotting when the " - 'default backend "matplotlib" is selected.' - ) - else: - return + from pandas.plotting._core import _get_plot_backend - try: - importlib.import_module(backend_str) - except ImportError: - raise ValueError( - '"{}" does not seem to be an installed module. ' - "A pandas plotting backend must be a module that " - "can be imported".format(backend_str) - ) + backend_str = cf.get_option(key) + _get_plot_backend(backend_str) with cf.config_prefix("plotting"): diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index 2e6a401b49efc..33c50eb72f4dd 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -1576,10 +1576,18 @@ def _find_backend(backend: str): # We re-raise later on. pass else: - _backends[backend] = module - return module - - raise ValueError("No backend {}".format(backend)) + if hasattr(module, "plot"): + # Validate that the interface is implemented when the option + # is set, rather than at plot time. + _backends[backend] = module + return module + + msg = ( + "Could not find plotting backend '{name}'. Ensure that you've installed the " + "package providing the '{name}' entrypoint, or that the package has a" + "top-level `.plot` method." + ) + raise ValueError(msg.format(name=backend)) def _get_plot_backend(backend=None): @@ -1600,7 +1608,13 @@ def _get_plot_backend(backend=None): if backend == "matplotlib": # Because matplotlib is an optional dependency and first-party backend, # we need to attempt an import here to raise an ImportError if needed. - import pandas.plotting._matplotlib as module + try: + import pandas.plotting._matplotlib as module + except ImportError: + raise ImportError( + "matplotlib is required for plotting when the " + "default backend 'matplotlib' is selected." + ) from None _backends["matplotlib"] = module diff --git a/pandas/tests/plotting/test_backend.py b/pandas/tests/plotting/test_backend.py index d126407cfd823..3c1909948d389 100644 --- a/pandas/tests/plotting/test_backend.py +++ b/pandas/tests/plotting/test_backend.py @@ -9,6 +9,17 @@ import pandas +@pytest.fixture +def dummy_backend(): + backend = types.ModuleType("pandas_dummy_backend") + backend.plot = lambda *args, **kwargs: None + sys.modules["pandas_dummy_backend"] = backend + + yield + + del sys.modules["pandas_dummy_backend"] + + def test_matplotlib_backend_error(): msg = ( "matplotlib is required for plotting when the default backend " @@ -22,20 +33,14 @@ def test_matplotlib_backend_error(): def test_backend_is_not_module(): - msg = ( - '"not_an_existing_module" does not seem to be an installed module. ' - "A pandas plotting backend must be a module that can be imported" - ) + msg = "Could not find plotting backend 'not_an_existing_module'." with pytest.raises(ValueError, match=msg): pandas.set_option("plotting.backend", "not_an_existing_module") -def test_backend_is_correct(monkeypatch): - monkeypatch.setattr( - "pandas.core.config_init.importlib.import_module", lambda name: None - ) - pandas.set_option("plotting.backend", "correct_backend") - assert pandas.get_option("plotting.backend") == "correct_backend" +def test_backend_is_correct(dummy_backend): + pandas.set_option("plotting.backend", "pandas_dummy_backend") + assert pandas.get_option("plotting.backend") == "pandas_dummy_backend" # Restore backend for other tests (matplotlib can be not installed) try: @@ -74,6 +79,16 @@ def test_register_entrypoint(): assert result is mod +def test_setting_backend_raies(): + module = types.ModuleType("pandas_plot_backend") + sys.modules["pandas_plot_backend"] = module + + with pytest.raises( + ValueError, match="Could not find plotting backend 'pandas_plot_backend'." + ): + pandas.set_option("plotting.backend", "pandas_plot_backend") + + def test_register_import(): mod = types.ModuleType("my_backend2") mod.plot = lambda *args, **kwargs: 1 From f74c4c861d444577b298f923de76182a7b5a34bd Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 26 Aug 2019 21:46:16 -0500 Subject: [PATCH 2/6] fixups --- pandas/plotting/_core.py | 2 +- pandas/tests/plotting/test_backend.py | 17 ++++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index 33c50eb72f4dd..d3c9e8ccfa51c 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -1613,7 +1613,7 @@ def _get_plot_backend(backend=None): except ImportError: raise ImportError( "matplotlib is required for plotting when the " - "default backend 'matplotlib' is selected." + 'default backend "matplotlib" is selected.' ) from None _backends["matplotlib"] = module diff --git a/pandas/tests/plotting/test_backend.py b/pandas/tests/plotting/test_backend.py index 3c1909948d389..ee2d7d31318bf 100644 --- a/pandas/tests/plotting/test_backend.py +++ b/pandas/tests/plotting/test_backend.py @@ -15,7 +15,7 @@ def dummy_backend(): backend.plot = lambda *args, **kwargs: None sys.modules["pandas_dummy_backend"] = backend - yield + yield backend del sys.modules["pandas_dummy_backend"] @@ -41,6 +41,9 @@ def test_backend_is_not_module(): def test_backend_is_correct(dummy_backend): pandas.set_option("plotting.backend", "pandas_dummy_backend") assert pandas.get_option("plotting.backend") == "pandas_dummy_backend" + assert ( + pandas.plotting._core._get_plot_backend("pandas_dummy_backend") is dummy_backend + ) # Restore backend for other tests (matplotlib can be not installed) try: @@ -79,7 +82,8 @@ def test_register_entrypoint(): assert result is mod -def test_setting_backend_raies(): +def test_setting_backend_without_plot_raies(): + # GH-28163 module = types.ModuleType("pandas_plot_backend") sys.modules["pandas_plot_backend"] = module @@ -89,15 +93,6 @@ def test_setting_backend_raies(): pandas.set_option("plotting.backend", "pandas_plot_backend") -def test_register_import(): - mod = types.ModuleType("my_backend2") - mod.plot = lambda *args, **kwargs: 1 - sys.modules["my_backend2"] = mod - - result = pandas.plotting._core._get_plot_backend("my_backend2") - assert result is mod - - @td.skip_if_mpl def test_no_matplotlib_ok(): with pytest.raises(ImportError): From 63b237dfce19c110c8682204afc859827022780c Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 27 Aug 2019 10:20:14 -0500 Subject: [PATCH 3/6] wip --- pandas/core/config_init.py | 6 +++-- pandas/plotting/_core.py | 4 +++- pandas/tests/plotting/test_backend.py | 34 ++++++++++++++------------- pandas/tests/plotting/test_misc.py | 2 +- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/pandas/core/config_init.py b/pandas/core/config_init.py index 276f5d8478beb..e3ca328c51665 100644 --- a/pandas/core/config_init.py +++ b/pandas/core/config_init.py @@ -581,6 +581,8 @@ def use_inf_as_na_cb(key): def register_plotting_backend_cb(key): from pandas.plotting._core import _get_plot_backend + # import pdb; pdb.set_trace() + backend_str = cf.get_option(key) _get_plot_backend(backend_str) @@ -590,8 +592,8 @@ def register_plotting_backend_cb(key): "backend", defval="matplotlib", doc=plotting_backend_doc, - validator=str, - cb=register_plotting_backend_cb, + # validator=str, + validator=register_plotting_backend_cb, ) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index d3c9e8ccfa51c..1b3a31fb54577 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -15,8 +15,10 @@ # we can lazily import matplotlib. try: import pandas.plotting._matplotlib # noqa + + _HAS_MATPLOTLIB = True except ImportError: - pass + _HAS_MATPLOTLIB = False def hist_series( diff --git a/pandas/tests/plotting/test_backend.py b/pandas/tests/plotting/test_backend.py index ee2d7d31318bf..36b4d6c60506d 100644 --- a/pandas/tests/plotting/test_backend.py +++ b/pandas/tests/plotting/test_backend.py @@ -8,9 +8,17 @@ import pandas +from pandas.plotting._core import _HAS_MATPLOTLIB + + +@pytest.fixture +def restore_backend(): + if _HAS_MATPLOTLIB: + pandas.set_option("plotting.backend", "matplotlib") + @pytest.fixture -def dummy_backend(): +def dummy_backend(restore_backend): backend = types.ModuleType("pandas_dummy_backend") backend.plot = lambda *args, **kwargs: None sys.modules["pandas_dummy_backend"] = backend @@ -20,16 +28,14 @@ def dummy_backend(): del sys.modules["pandas_dummy_backend"] +@td.skip_if_mpl() def test_matplotlib_backend_error(): msg = ( "matplotlib is required for plotting when the default backend " '"matplotlib" is selected.' ) - try: - import matplotlib # noqa - except ImportError: - with pytest.raises(ImportError, match=msg): - pandas.set_option("plotting.backend", "matplotlib") + with pytest.raises(ImportError, match=msg): + pandas.set_option("plotting.backend", "matplotlib") def test_backend_is_not_module(): @@ -45,15 +51,9 @@ def test_backend_is_correct(dummy_backend): pandas.plotting._core._get_plot_backend("pandas_dummy_backend") is dummy_backend ) - # Restore backend for other tests (matplotlib can be not installed) - try: - pandas.set_option("plotting.backend", "matplotlib") - except ImportError: - pass - -@td.skip_if_no_mpl -def test_register_entrypoint(): +@td.skip_if_no_mpl() +def test_register_entrypoint(restore_backend): dist = pkg_resources.get_distribution("pandas") if dist.module_path not in pandas.__file__: @@ -82,7 +82,7 @@ def test_register_entrypoint(): assert result is mod -def test_setting_backend_without_plot_raies(): +def test_setting_backend_without_plot_raises(): # GH-28163 module = types.ModuleType("pandas_plot_backend") sys.modules["pandas_plot_backend"] = module @@ -92,8 +92,10 @@ def test_setting_backend_without_plot_raies(): ): pandas.set_option("plotting.backend", "pandas_plot_backend") + assert pandas.options.plotting.backend == "matplotlib" + -@td.skip_if_mpl +@td.skip_if_mpl() def test_no_matplotlib_ok(): with pytest.raises(ImportError): pandas.plotting._core._get_plot_backend("matplotlib") diff --git a/pandas/tests/plotting/test_misc.py b/pandas/tests/plotting/test_misc.py index 6cb6f818d40fd..940cfef4058e0 100644 --- a/pandas/tests/plotting/test_misc.py +++ b/pandas/tests/plotting/test_misc.py @@ -21,7 +21,7 @@ def test_import_error_message(): # GH-19810 df = DataFrame({"A": [1, 2]}) - with pytest.raises(ImportError, match="No module named 'matplotlib'"): + with pytest.raises(ImportError, match="matplotlib is required for plotting"): df.plot() From e72bda764908790a48a755383bc3ab7d1107a523 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 28 Aug 2019 15:37:15 -0500 Subject: [PATCH 4/6] fixup --- pandas/core/config_init.py | 8 ++++---- pandas/tests/plotting/test_backend.py | 27 +++++++++++++++------------ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/pandas/core/config_init.py b/pandas/core/config_init.py index e3ca328c51665..9596b2c06c9a2 100644 --- a/pandas/core/config_init.py +++ b/pandas/core/config_init.py @@ -579,12 +579,12 @@ def use_inf_as_na_cb(key): def register_plotting_backend_cb(key): + if key == "matplotlib": + # We defer matplotlib validation, since it's the default + return from pandas.plotting._core import _get_plot_backend - # import pdb; pdb.set_trace() - - backend_str = cf.get_option(key) - _get_plot_backend(backend_str) + _get_plot_backend(key) with cf.config_prefix("plotting"): diff --git a/pandas/tests/plotting/test_backend.py b/pandas/tests/plotting/test_backend.py index 36b4d6c60506d..e5b9fa094d1ff 100644 --- a/pandas/tests/plotting/test_backend.py +++ b/pandas/tests/plotting/test_backend.py @@ -8,13 +8,13 @@ import pandas -from pandas.plotting._core import _HAS_MATPLOTLIB - @pytest.fixture def restore_backend(): - if _HAS_MATPLOTLIB: - pandas.set_option("plotting.backend", "matplotlib") + """Restore the plotting backend to matplotlib""" + pandas.set_option("plotting.backend", "matplotlib") + yield + pandas.set_option("plotting.backend", "matplotlib") @pytest.fixture @@ -28,14 +28,14 @@ def dummy_backend(restore_backend): del sys.modules["pandas_dummy_backend"] -@td.skip_if_mpl() -def test_matplotlib_backend_error(): - msg = ( - "matplotlib is required for plotting when the default backend " - '"matplotlib" is selected.' - ) - with pytest.raises(ImportError, match=msg): - pandas.set_option("plotting.backend", "matplotlib") +# @td.skip_if_mpl() +# def test_matplotlib_backend_error(): +# msg = ( +# "matplotlib is required for plotting when the default backend " +# '"matplotlib" is selected.' +# ) +# with pytest.raises(ImportError, match=msg): +# pandas.set_option("plotting.backend", "matplotlib") def test_backend_is_not_module(): @@ -43,6 +43,8 @@ def test_backend_is_not_module(): with pytest.raises(ValueError, match=msg): pandas.set_option("plotting.backend", "not_an_existing_module") + assert pandas.options.plotting.backend == "matplotlib" + def test_backend_is_correct(dummy_backend): pandas.set_option("plotting.backend", "pandas_dummy_backend") @@ -87,6 +89,7 @@ def test_setting_backend_without_plot_raises(): module = types.ModuleType("pandas_plot_backend") sys.modules["pandas_plot_backend"] = module + assert pandas.options.plotting.backend == "matplotlib" with pytest.raises( ValueError, match="Could not find plotting backend 'pandas_plot_backend'." ): From db5245973a11c077aebe9bbf3448d2ef2d746442 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 28 Aug 2019 15:40:30 -0500 Subject: [PATCH 5/6] cleanup --- pandas/core/config_init.py | 1 - pandas/plotting/_core.py | 4 +--- pandas/tests/plotting/test_backend.py | 14 ++------------ 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/pandas/core/config_init.py b/pandas/core/config_init.py index 9596b2c06c9a2..dfc80140433f8 100644 --- a/pandas/core/config_init.py +++ b/pandas/core/config_init.py @@ -592,7 +592,6 @@ def register_plotting_backend_cb(key): "backend", defval="matplotlib", doc=plotting_backend_doc, - # validator=str, validator=register_plotting_backend_cb, ) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index 1b3a31fb54577..d3c9e8ccfa51c 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -15,10 +15,8 @@ # we can lazily import matplotlib. try: import pandas.plotting._matplotlib # noqa - - _HAS_MATPLOTLIB = True except ImportError: - _HAS_MATPLOTLIB = False + pass def hist_series( diff --git a/pandas/tests/plotting/test_backend.py b/pandas/tests/plotting/test_backend.py index e5b9fa094d1ff..97c0be04f5697 100644 --- a/pandas/tests/plotting/test_backend.py +++ b/pandas/tests/plotting/test_backend.py @@ -28,16 +28,6 @@ def dummy_backend(restore_backend): del sys.modules["pandas_dummy_backend"] -# @td.skip_if_mpl() -# def test_matplotlib_backend_error(): -# msg = ( -# "matplotlib is required for plotting when the default backend " -# '"matplotlib" is selected.' -# ) -# with pytest.raises(ImportError, match=msg): -# pandas.set_option("plotting.backend", "matplotlib") - - def test_backend_is_not_module(): msg = "Could not find plotting backend 'not_an_existing_module'." with pytest.raises(ValueError, match=msg): @@ -54,7 +44,7 @@ def test_backend_is_correct(dummy_backend): ) -@td.skip_if_no_mpl() +@td.skip_if_no_mpl def test_register_entrypoint(restore_backend): dist = pkg_resources.get_distribution("pandas") @@ -98,7 +88,7 @@ def test_setting_backend_without_plot_raises(): assert pandas.options.plotting.backend == "matplotlib" -@td.skip_if_mpl() +@td.skip_if_mpl def test_no_matplotlib_ok(): with pytest.raises(ImportError): pandas.plotting._core._get_plot_backend("matplotlib") From 44192835a3f9d291828499a33b7db624b8fd5519 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Fri, 30 Aug 2019 14:42:23 -0500 Subject: [PATCH 6/6] patch --- pandas/tests/plotting/test_backend.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/pandas/tests/plotting/test_backend.py b/pandas/tests/plotting/test_backend.py index 97c0be04f5697..6511d94aa4c09 100644 --- a/pandas/tests/plotting/test_backend.py +++ b/pandas/tests/plotting/test_backend.py @@ -8,6 +8,9 @@ import pandas +dummy_backend = types.ModuleType("pandas_dummy_backend") +dummy_backend.plot = lambda *args, **kwargs: None + @pytest.fixture def restore_backend(): @@ -17,17 +20,6 @@ def restore_backend(): pandas.set_option("plotting.backend", "matplotlib") -@pytest.fixture -def dummy_backend(restore_backend): - backend = types.ModuleType("pandas_dummy_backend") - backend.plot = lambda *args, **kwargs: None - sys.modules["pandas_dummy_backend"] = backend - - yield backend - - del sys.modules["pandas_dummy_backend"] - - def test_backend_is_not_module(): msg = "Could not find plotting backend 'not_an_existing_module'." with pytest.raises(ValueError, match=msg): @@ -36,7 +28,9 @@ def test_backend_is_not_module(): assert pandas.options.plotting.backend == "matplotlib" -def test_backend_is_correct(dummy_backend): +def test_backend_is_correct(monkeypatch, restore_backend): + monkeypatch.setitem(sys.modules, "pandas_dummy_backend", dummy_backend) + pandas.set_option("plotting.backend", "pandas_dummy_backend") assert pandas.get_option("plotting.backend") == "pandas_dummy_backend" assert (