From 28412d2f8e5acdf5ffd52dcd662169047fa09f8a Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 8 Aug 2024 13:41:39 +0800 Subject: [PATCH 01/13] Refactor and add the _get_default_display_method function --- pygmt/figure.py | 49 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/pygmt/figure.py b/pygmt/figure.py index ce8693a43f3..7e503667a17 100644 --- a/pygmt/figure.py +++ b/pygmt/figure.py @@ -6,6 +6,7 @@ import os from pathlib import Path from tempfile import TemporaryDirectory +from typing import Literal try: import IPython @@ -29,23 +30,43 @@ # A registry of all figures that have had "show" called in this session. # This is needed for the sphinx-gallery scraper in pygmt/sphinx_gallery.py SHOWED_FIGURES = [] - -# Configurations for figure display +# Configurations for figure display. SHOW_CONFIG = { - "method": "external", # Open in an external viewer by default + "method": None, # The image preview display method. } -# Show figures in Jupyter notebooks if available -if _HAS_IPYTHON: - get_ipython = IPython.get_ipython() - if get_ipython and "IPKernelApp" in get_ipython.config: # Jupyter Notebook enabled - SHOW_CONFIG["method"] = "notebook" -# Set environment variable PYGMT_USE_EXTERNAL_DISPLAY to 'false' to disable -# external display. Use it when running the tests and building the docs to -# avoid popping up windows. -if os.environ.get("PYGMT_USE_EXTERNAL_DISPLAY", "true").lower() == "false": - SHOW_CONFIG["method"] = "none" +def _get_default_display_method() -> Literal["external", "notebook", "none"]: + """ + Get the default method to display preview images. + + The function checks the current environment and determine the most suitable method + to display preview images when calling :meth:`pygmt.Figure.show`. Valid display + methods are: + + - ``"external"``: External PDF preview using the default PDF viewer + - ``"notebook"``: Inline PNG preview in the current notebook + - ``"none"``: Disable image preview + + The default display method is ``"notebook"`` in Jupyter notebook environment, and + ``"external"`` in other cases. + + Setting environment variable **PYGMT_USE_EXTERNAL_DISPLAY** to ``"false"`` can + disable image preview. It's useful when running the tests and building the + documentation to avoid popping up windows. + + Returns + ------- + method + The default display method. + """ + if os.environ.get("PYGMT_USE_EXTERNAL_DISPLAY", "true").lower() == "false": + return "none" + if _HAS_IPYTHON: + get_ipython = IPython.get_ipython() + if get_ipython and "IPKernelApp" in get_ipython.config: + return "notebook" + return "external" class Figure: @@ -442,6 +463,8 @@ def show(self, dpi=300, width=500, method=None, waiting=0.5, **kwargs): # Set the display method if method is None: + if SHOW_CONFIG["method"] is None: + SHOW_CONFIG["method"] = _get_default_display_method() method = SHOW_CONFIG["method"] if method not in {"external", "notebook", "none"}: From 9468ec8fb350490f6d38e88e1ffdae29b676f134 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 28 Aug 2024 13:53:19 +0800 Subject: [PATCH 02/13] Update pygmt/figure.py Co-authored-by: Michael Grund <23025878+michaelgrund@users.noreply.github.com> --- pygmt/figure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygmt/figure.py b/pygmt/figure.py index 7e503667a17..53f77b8cc0e 100644 --- a/pygmt/figure.py +++ b/pygmt/figure.py @@ -48,7 +48,7 @@ def _get_default_display_method() -> Literal["external", "notebook", "none"]: - ``"notebook"``: Inline PNG preview in the current notebook - ``"none"``: Disable image preview - The default display method is ``"notebook"`` in Jupyter notebook environment, and + The default display method is ``"notebook"`` in the Jupyter notebook environment, and ``"external"`` in other cases. Setting environment variable **PYGMT_USE_EXTERNAL_DISPLAY** to ``"false"`` can From 4d6b46b09e56082272c677f0a95084cbe76f7c07 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 28 Aug 2024 17:05:40 +0800 Subject: [PATCH 03/13] PYGMT_USE_EXTERNAL_DISPLAY should only affect external display --- pygmt/figure.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pygmt/figure.py b/pygmt/figure.py index 53f77b8cc0e..991a0050ef5 100644 --- a/pygmt/figure.py +++ b/pygmt/figure.py @@ -48,8 +48,8 @@ def _get_default_display_method() -> Literal["external", "notebook", "none"]: - ``"notebook"``: Inline PNG preview in the current notebook - ``"none"``: Disable image preview - The default display method is ``"notebook"`` in the Jupyter notebook environment, and - ``"external"`` in other cases. + The default display method is ``"notebook"`` in the Jupyter notebook environment, + and ``"external"`` in other cases. Setting environment variable **PYGMT_USE_EXTERNAL_DISPLAY** to ``"false"`` can disable image preview. It's useful when running the tests and building the @@ -60,12 +60,12 @@ def _get_default_display_method() -> Literal["external", "notebook", "none"]: method The default display method. """ - if os.environ.get("PYGMT_USE_EXTERNAL_DISPLAY", "true").lower() == "false": - return "none" if _HAS_IPYTHON: get_ipython = IPython.get_ipython() if get_ipython and "IPKernelApp" in get_ipython.config: return "notebook" + if os.environ.get("PYGMT_USE_EXTERNAL_DISPLAY", "true").lower() == "false": + return "none" return "external" From e3280921817c4bb8b453cd13fc7505451c6bcff5 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 28 Aug 2024 19:08:52 +0800 Subject: [PATCH 04/13] Refactor to init the display method at import time --- pygmt/figure.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/pygmt/figure.py b/pygmt/figure.py index 991a0050ef5..86b46d86015 100644 --- a/pygmt/figure.py +++ b/pygmt/figure.py @@ -27,14 +27,6 @@ use_alias, ) -# A registry of all figures that have had "show" called in this session. -# This is needed for the sphinx-gallery scraper in pygmt/sphinx_gallery.py -SHOWED_FIGURES = [] -# Configurations for figure display. -SHOW_CONFIG = { - "method": None, # The image preview display method. -} - def _get_default_display_method() -> Literal["external", "notebook", "none"]: """ @@ -69,6 +61,15 @@ def _get_default_display_method() -> Literal["external", "notebook", "none"]: return "external" +# A registry of all figures that have had "show" called in this session. +# This is needed for the sphinx-gallery scraper in pygmt/sphinx_gallery.py +SHOWED_FIGURES = [] +# Configurations for figure display. +SHOW_CONFIG = { + "method": _get_default_display_method(), # The image preview display method. +} + + class Figure: """ A GMT figure to handle all plotting. @@ -463,8 +464,6 @@ def show(self, dpi=300, width=500, method=None, waiting=0.5, **kwargs): # Set the display method if method is None: - if SHOW_CONFIG["method"] is None: - SHOW_CONFIG["method"] = _get_default_display_method() method = SHOW_CONFIG["method"] if method not in {"external", "notebook", "none"}: From 334e9c4e3ebfebc55ec49c3fd2827c51127aa9e0 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 28 Aug 2024 19:33:28 +0800 Subject: [PATCH 05/13] Add some tests --- pygmt/tests/test_figure.py | 54 +++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/pygmt/tests/test_figure.py b/pygmt/tests/test_figure.py index 042c9876e71..0b036a15bd4 100644 --- a/pygmt/tests/test_figure.py +++ b/pygmt/tests/test_figure.py @@ -4,7 +4,6 @@ Doesn't include the plotting commands which have their own test files. """ -import importlib from pathlib import Path import numpy as np @@ -12,9 +11,15 @@ import pytest from pygmt import Figure, set_display from pygmt.exceptions import GMTError, GMTInvalidInput +from pygmt.figure import _get_default_display_method from pygmt.helpers import GMTTempFile -HAS_IPYTHON = bool(importlib.util.find_spec("IPython")) +try: + import IPython + + _HAS_IPYTHON = True +except ImportError: + _HAS_IPYTHON = False def test_figure_region(): @@ -307,7 +312,7 @@ def test_figure_savefig_worldfile(): fig.savefig(fname=imgfile.name, worldfile=True) -@pytest.mark.skipif(not HAS_IPYTHON, reason="run when IPython is installed") +@pytest.mark.skipif(not _HAS_IPYTHON, reason="run when IPython is installed") def test_figure_show(): """ Test that show creates the correct file name and deletes the temp dir. @@ -347,7 +352,7 @@ def test_figure_show_invalid_method(): fig.show(method="test") -@pytest.mark.skipif(HAS_IPYTHON, reason="run without IPython installed") +@pytest.mark.skipif(_HAS_IPYTHON, reason="run without IPython installed") def test_figure_show_notebook_error_without_ipython(): """ Test to check if an error is raised when display method is 'notebook', but IPython @@ -390,3 +395,44 @@ def test_figure_unsupported_xshift_yshift(): fig.plot(x=1, y=1, style="c3c", yshift="3c") with pytest.raises(GMTInvalidInput): fig.plot(x=1, y=1, style="c3c", Y="3c") + + +class TestGetDefaultDisplayMethod: + """ + Test the _get_default_display_method function. + """ + + def test_default_display_method(self): + """ + The default display method is "external" as tests are not run in IPython. + """ + assert _get_default_display_method() == "external" + + def test_disable_external_display(self, monkeypatch): + """ + Set PYGMT_USE_EXTERNAL_DISPLAY to "false" should disable external display. + """ + monkeypatch.setenv("PYGMT_USE_EXTERNAL_DISPLAY", "false") + assert _get_default_display_method() == "none" + + def test_notebook_display(self, monkeypatch): + """ + The default display method is "notebook" when IPython instance is available. + """ + + class MockIPython: + """ + A simple mock class to simulate an IPython instance. + """ + + def __init__(self): + self.config = {"IPKernelApp": True} + + mock_ipython = MockIPython() + + monkeypatch.setattr(IPython, "get_ipython", lambda: mock_ipython) + assert _get_default_display_method() == "notebook" + + # PYGMT_USE_EXTERNAL_DISPLAY should not affect notebook display. + monkeypatch.setenv("PYGMT_USE_EXTERNAL_DISPLAY", "false") + assert _get_default_display_method() == "notebook" From 20d49ba8dadb94a40bdc2f3082b80465bfc0cb0b Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 28 Aug 2024 19:42:19 +0800 Subject: [PATCH 06/13] Fix tests --- pygmt/tests/test_figure.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pygmt/tests/test_figure.py b/pygmt/tests/test_figure.py index 0b036a15bd4..f8cc5daf890 100644 --- a/pygmt/tests/test_figure.py +++ b/pygmt/tests/test_figure.py @@ -402,10 +402,11 @@ class TestGetDefaultDisplayMethod: Test the _get_default_display_method function. """ - def test_default_display_method(self): + def test_default_display_method(self, monkeypatch): """ The default display method is "external" as tests are not run in IPython. """ + monkeypatch.setenv("PYGMT_USE_EXTERNAL_DISPLAY", "true") assert _get_default_display_method() == "external" def test_disable_external_display(self, monkeypatch): @@ -415,6 +416,7 @@ def test_disable_external_display(self, monkeypatch): monkeypatch.setenv("PYGMT_USE_EXTERNAL_DISPLAY", "false") assert _get_default_display_method() == "none" + @pytest.mark.skipif(not _HAS_IPYTHON, reason="Run when IPython is installed") def test_notebook_display(self, monkeypatch): """ The default display method is "notebook" when IPython instance is available. From d28a25df76a4dba88010538e67f6252470b5b33c Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 28 Aug 2024 21:09:38 +0800 Subject: [PATCH 07/13] Update comments --- pygmt/tests/test_figure.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pygmt/tests/test_figure.py b/pygmt/tests/test_figure.py index f8cc5daf890..168f32f9777 100644 --- a/pygmt/tests/test_figure.py +++ b/pygmt/tests/test_figure.py @@ -404,14 +404,14 @@ class TestGetDefaultDisplayMethod: def test_default_display_method(self, monkeypatch): """ - The default display method is "external" as tests are not run in IPython. + Default display method is "external" if PYGMT_USE_EXTERNAL_DISPLAY is undefined. """ - monkeypatch.setenv("PYGMT_USE_EXTERNAL_DISPLAY", "true") + monkeypatch.delenv("PYGMT_USE_EXTERNAL_DISPLAY", raising=False) assert _get_default_display_method() == "external" def test_disable_external_display(self, monkeypatch): """ - Set PYGMT_USE_EXTERNAL_DISPLAY to "false" should disable external display. + Setting PYGMT_USE_EXTERNAL_DISPLAY to "false" should disable external display. """ monkeypatch.setenv("PYGMT_USE_EXTERNAL_DISPLAY", "false") assert _get_default_display_method() == "none" @@ -419,7 +419,7 @@ def test_disable_external_display(self, monkeypatch): @pytest.mark.skipif(not _HAS_IPYTHON, reason="Run when IPython is installed") def test_notebook_display(self, monkeypatch): """ - The default display method is "notebook" when IPython instance is available. + Default display method is "notebook" when an IPython kernel is running. """ class MockIPython: From 77b378d315b91b482640eb3fdcb299179a313d86 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 28 Aug 2024 21:12:01 +0800 Subject: [PATCH 08/13] Improve _get_default_display_method docstrings --- pygmt/figure.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pygmt/figure.py b/pygmt/figure.py index 86b46d86015..5c4a5c3572f 100644 --- a/pygmt/figure.py +++ b/pygmt/figure.py @@ -32,7 +32,7 @@ def _get_default_display_method() -> Literal["external", "notebook", "none"]: """ Get the default method to display preview images. - The function checks the current environment and determine the most suitable method + The function checks the current environment and determines the most suitable method to display preview images when calling :meth:`pygmt.Figure.show`. Valid display methods are: @@ -44,8 +44,8 @@ def _get_default_display_method() -> Literal["external", "notebook", "none"]: and ``"external"`` in other cases. Setting environment variable **PYGMT_USE_EXTERNAL_DISPLAY** to ``"false"`` can - disable image preview. It's useful when running the tests and building the - documentation to avoid popping up windows. + disable image preview in external viewers. It's useful when running the tests and + building the documentation to avoid popping up windows. Returns ------- From 9afb724514fddcdc38deb6b043a86f95c7387569 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 28 Aug 2024 21:14:17 +0800 Subject: [PATCH 09/13] Improve test comments --- pygmt/tests/test_figure.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pygmt/tests/test_figure.py b/pygmt/tests/test_figure.py index 168f32f9777..aadaaacfad3 100644 --- a/pygmt/tests/test_figure.py +++ b/pygmt/tests/test_figure.py @@ -430,9 +430,11 @@ class MockIPython: def __init__(self): self.config = {"IPKernelApp": True} + # Mock IPython.get_ipython() to return a MockIPython instance. mock_ipython = MockIPython() - monkeypatch.setattr(IPython, "get_ipython", lambda: mock_ipython) + + # Default display method should be "notebook" when an IPython kernel is running. assert _get_default_display_method() == "notebook" # PYGMT_USE_EXTERNAL_DISPLAY should not affect notebook display. From 83288c3d80e94200ac016c52824b51aafe4365e3 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 30 Aug 2024 07:56:54 +0800 Subject: [PATCH 10/13] Simplify the code block for checking IPython kernels --- pygmt/figure.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pygmt/figure.py b/pygmt/figure.py index 8f3303a255f..05cba50ed02 100644 --- a/pygmt/figure.py +++ b/pygmt/figure.py @@ -52,12 +52,13 @@ def _get_default_display_method() -> Literal["external", "notebook", "none"]: method The default display method. """ - if _HAS_IPYTHON: - get_ipython = IPython.get_ipython() - if get_ipython and "IPKernelApp" in get_ipython.config: - return "notebook" + # Check if an IPython kernel is running. + if _HAS_IPYTHON and (ipy := IPython.get_ipython()) and "IPKernelApp" in ipy.config: + return "notebook" + # Check if the environment variable PYGMT_USE_EXTERNAL_DISPLAY is set to "false". if os.environ.get("PYGMT_USE_EXTERNAL_DISPLAY", "true").lower() == "false": return "none" + # Fallback to using the external viewer. return "external" From 0045fa5c351601bdc43179226dd995b3a448cbe9 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 8 Aug 2024 13:25:36 +0800 Subject: [PATCH 11/13] pygmt.set_display: Fix a bug when setting `method=None`, add type hints and refactor --- pygmt/figure.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/pygmt/figure.py b/pygmt/figure.py index 05cba50ed02..9ad6f737553 100644 --- a/pygmt/figure.py +++ b/pygmt/figure.py @@ -562,22 +562,20 @@ def _repr_html_(self): ) -def set_display(method=None): +def set_display(method: Literal["external", "notebook", "none", None] = None): """ Set the display method when calling :meth:`pygmt.Figure.show`. Parameters ---------- - method : str or None + method The method to display an image preview. Choose from: - ``"external"``: External PDF preview using the default PDF viewer - ``"notebook"``: Inline PNG preview in the current notebook - ``"none"``: Disable image preview - - ``None``: Reset to the default display method - - The default display method is ``"external"`` in Python consoles or - ``"notebook"`` in Jupyter notebooks. + - ``None``: Reset to the default display method, which is either ``"external"`` + in Python consoles or ``"notebook"`` in Jupyter notebooks. Examples -------- @@ -600,10 +598,13 @@ def set_display(method=None): >>> pygmt.set_display(method=None) >>> fig.show() # again, will show a PNG image in the current notebook """ - if method in {"notebook", "external", "none"}: - SHOW_CONFIG["method"] = method - elif method is not None: - raise GMTInvalidInput( - f"Invalid display mode '{method}', " - "should be either 'notebook', 'external', 'none' or None." - ) + match method: + case "external" | "notebook" | "none": + SHOW_CONFIG["method"] = method # type: ignore[assignment] + case None: + SHOW_CONFIG["method"] = _get_default_display_method() # type: ignore[assignment] + case _: + raise GMTInvalidInput( + f"Invalid display method '{method}'. Valid values are 'external'," + "'notebook', 'none' or None." + ) From 7ea264e61c4d60e0121676c3d8538caa81812e06 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 30 Aug 2024 08:14:42 +0800 Subject: [PATCH 12/13] Add one test --- pygmt/tests/test_figure.py | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/pygmt/tests/test_figure.py b/pygmt/tests/test_figure.py index aadaaacfad3..8c1c03dcbe6 100644 --- a/pygmt/tests/test_figure.py +++ b/pygmt/tests/test_figure.py @@ -11,7 +11,7 @@ import pytest from pygmt import Figure, set_display from pygmt.exceptions import GMTError, GMTInvalidInput -from pygmt.figure import _get_default_display_method +from pygmt.figure import SHOW_CONFIG, _get_default_display_method from pygmt.helpers import GMTTempFile try: @@ -373,12 +373,28 @@ def test_figure_display_external(): fig.show(method="external") -def test_figure_set_display_invalid(): +class TestSetDisplay: """ - Test to check if an error is raised when an invalid method is passed to set_display. + Test the pygmt.set_display method. """ - with pytest.raises(GMTInvalidInput): - set_display(method="invalid") + + def test_set_display(self): + """ + Test pygmt.set_display. + """ + current_method = SHOW_CONFIG["method"] + for method in ("notebook", "external", "none"): + set_display(method=method) + assert SHOW_CONFIG["method"] == method + set_display(method=None) + assert SHOW_CONFIG["method"] == current_method + + def test_invalid_method(self): + """ + Test if an error is raised when an invalid method is passed. + """ + with pytest.raises(GMTInvalidInput): + set_display(method="invalid") def test_figure_unsupported_xshift_yshift(): From f6bce0ac9d18e29de7186791bf9d3234916b2a0b Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 30 Aug 2024 08:17:08 +0800 Subject: [PATCH 13/13] Improve tests --- pygmt/tests/test_figure.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pygmt/tests/test_figure.py b/pygmt/tests/test_figure.py index 8c1c03dcbe6..4c8e184d118 100644 --- a/pygmt/tests/test_figure.py +++ b/pygmt/tests/test_figure.py @@ -380,14 +380,17 @@ class TestSetDisplay: def test_set_display(self): """ - Test pygmt.set_display. + Test if pygmt.set_display updates the SHOW_CONFIG variable correctly. """ - current_method = SHOW_CONFIG["method"] + default_method = SHOW_CONFIG["method"] # Current default method + for method in ("notebook", "external", "none"): set_display(method=method) assert SHOW_CONFIG["method"] == method + + # Setting method to None should revert it to the default method. set_display(method=None) - assert SHOW_CONFIG["method"] == current_method + assert SHOW_CONFIG["method"] == default_method def test_invalid_method(self): """