Repository: GenericMappingTools/pygmt

Issue:
Rethink the display mechanisms
**Description of the desired feature**

Currently, there many ways of displaying a PyGMT figure:

* Calling `Figure.show()` in the notebook returns an IPython PNG display object for the jupyter notebook
* Putting the `Figure` object at the end of a notebook cell displays the figure as PNG
* Calling `Figure.show(external=True)` will open a preview in an external PDF viewer or the browser. The external viewer is non-blocking so this doesn't work for a script.
* Calling `Figure.savefig(..., show=True)` will save the figure and open it on an external viewer

This is not ideal. It would be better to have:

* Calling `Figure.show()` opens a preview of the figure and halts the Python process until the viewer is closed. It can also return `self` so that if it's called at the end of a notebook cell it would get displayed on the notebook as well.
* Putting the `Figure` at the end of a notebook cell displays a PNG preview.
* Having some way of tell `Figure.show()` to not open the viewer. This is required when using a notebook or when building the docs (we don't want windows opening when we build the gallery or tutorials). This can be done through a function `pygmt.enable_notebook` which would set a flag to tell `show` not to do anything and/or through an environment variable `PYGMT_DISPLAY` which can be set to `none` or other options that the user might want. Ideally, we should have both. The env variable will be useful when building the docs.



Discussion:
leouieda: @seisman thanks for pushing this forward! It would be good to have a manual method of setting the display since there is no guarantee that the notebook detection always works (and this is by design of jupyter). How about merging the two functions you have into `pygmt.set_display("notebook")` and `pygmt.set_display("external")`?
weiji14: cc @MarkWieczorek who made some comments in #366 (and this PR should ideally close it). Also might want to check out how `tqdm` does it at https://github.com/tqdm/tqdm#ipythonjupyter-integration, not sure if I like their `import` style but it might give you some ideas.

I'd also like to second @leouieda's comments about merging the two functions, since what we need is essentially a boolean True/False toggle (similar to the xarray accessor at #500). 
vercel[bot]: This pull request is being automatically deployed with Vercel ([learn more](https://vercel.link/github-learn-more)).  
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: [https://vercel.com/gmt/pygmt/kwrx6l4dx](https://vercel.com/gmt/pygmt/kwrx6l4dx)  
✅ Preview: [https://pygmt-git-new-display.gmt.vercel.app](https://pygmt-git-new-display.gmt.vercel.app)


seisman: @GenericMappingTools/python @GenericMappingTools/python-contributors @GenericMappingTools/python-maintainers @MarkWieczorek 

This PR changes the way PyGMT displays images in a "**backward-incompatible**" way. It fixes our two oldest issues #269 and #366. You can find more details by reading the long PR description.

Please help test the new design in different environments, give your comments and suggestions, and also try hard to break it and help improve it.
seisman: > I still think we can simplify this Pull Request right down to be focused solely on handling either `display="external"` or `display="notebook"`. Setting the dpi/width seems like a nice to have and out of scope of the original issue at #269 (wasn't mentioned at all in fact). 

Just made the changes to focus on the display method.

> (even better to just have a boolean `external=True/False` flag). 

PyGMT used `external=True/False` in the initial versions, and then decided to use `method="static/external"` when adding the support for NASA WorldWind Web (#132), although the WorldWind Web feature is removed later (#275).

I prefer to use `method="external"` so it's possible to add more display features in the future.

meghanrjones: I will try this out within the next couple days (I am even planning to set up a windows environment 😬) 
seisman: Please note that the current implemention is problematic. The following script only shows Figure 2 when running in one cell in Jupyter notebooks, because the return value of the code block is the Image object returned by `fig2.show()`:
```
import pygmt

fig1 = pygmt.Figure()
fig1.basemap(region=[0, 10, 0, 10], projection="X10c/10c", frame="WSrt+tFigure1")
fig1.show()

fig2 = pygmt.Figure()
fig2.basemap(region=[0, 10, 0, 10], projection="X10c/10c", frame="WSrt+tFigure2")
fig2.show()
```
So I may have to revert some changes and call `IPython.display.display()` instead, but it may cause more problem if we call `IPython.display.display()` and return `IPython.display.Image()`. 

Need some further investigation.
weiji14: > Please note that the current implemention is problematic. The following script only shows Figure 2 when running in one cell in Jupyter notebooks, because the return value of the code block is the Image object returned by `fig2.show()`:
> 
> ```
> import pygmt
> 
> fig1 = pygmt.Figure()
> fig1.basemap(region=[0, 10, 0, 10], projection="X10c/10c", frame="WSrt+tFigure1")
> fig1.show()
> 
> fig2 = pygmt.Figure()
> fig2.basemap(region=[0, 10, 0, 10], projection="X10c/10c", frame="WSrt+tFigure2")
> fig2.show()
> ```
> 
> So I may have to revert some changes and call `IPython.display.display()` instead, but it may cause more problem if we call `IPython.display.display()` and return `IPython.display.Image()`.
> 
> Edit: If we call `IPython.display.display()` and return `IPython.display.Image()`, and run the above script in one cell, we will see Figure 2 twice.

My knee-jerk thought is to just put those two code blocks in two different cells... Haven't used matplotlib in a while, but does the matplotlib `fig.show` method display two images when called that way?
seisman: > Haven't used matplotlib in a while, but does the matplotlib `fig.show` method display two images when called that way?

`plt.show()` returns `None`, and the following script shows two images.
```
import matplotlib.pyplot as plt

plt.plot([0, 1, 2], [2, 3, 4])
plt.show()

plt.plot([10, 11, 12], [12, 13, 14])
plt.show()
```

![image](https://user-images.githubusercontent.com/3974108/111921480-32011e80-8a6b-11eb-93b5-09d80c045bc4.png)

seisman: > For plotting the Image in another Jupyter notebook cell? Or plotting it again and again without having to call fig.show() (which calls psconvert). The IPython.display.Image object also has other methods (e.g. data which gives raw image data) that could be used.

Letting `Figure.show()` return an image sounds useful, but IMHO it brings more troubles and behaves inconsistently in different use cases. I believe Leo did the same thing in his original branch/PR (#528) due to the same reasons.

I think we should **NOT** let `Figure.show()` return the Image object. 

If someone wants to reuse the image, they can still save the image to disk and re-read it into memory.

Or we could add new a property to the Figure.class, for example `Figure.raw_image` which save the raw image data of the last `Figure.show()` call. 
seisman: @weiji14 Any more comments on this PR?
weiji14: > @weiji14 Any more comments on this PR?

I would prefer if someone else reviews this as well, e.g. @meghanrjones if she has time. I'm still somewhat on the fence about not returning the IPython.display.Image object as you recommend in https://github.com/GenericMappingTools/pygmt/pull/529#issuecomment-805508310, but will see what others say. Other than that, I have no major issues with the rest of the implementation in this PR.
meghanrjones: > > @weiji14 Any more comments on this PR?
> 
> I would prefer if someone else reviews this as well, e.g. @meghanrjones if she has time. I'm still somewhat on the fence about not returning the IPython.display.Image object as you recommend in [#529 (comment)](https://github.com/GenericMappingTools/pygmt/pull/529#issuecomment-805508310), but will see what others say. Other than that, I have no major issues with the rest of the implementation in this PR.

Yes, it's hard to believe so many days have passed since I said that I would review 😕. Sorry for that - I will work on this today unless Paul requests any 6.2 tasking.
meghanrjones: I am running into many problems with #1061, which is why I think it is necessary to merge #1062 first.
seisman: > Do you think that there should be warnings if dpi or width are used with method=external or method=none?

Sounds good to me. I will open a separate PR about dpi and width settings after this PR is merged.
seisman: > The TODO lists unit tests, is that still the plan or do you think this is good to go @seisman?

Actually, I have no idea how to test the new `Figure.show()` behavior automatically, because when running `make tests`, IPython is not imported so `Figure.show()` will do nothing.

Patch:
diff --git a/Makefile b/Makefile
index e5cae667..a5342aa8 100644
--- a/Makefile
+++ b/Makefile
@@ -36,7 +36,7 @@ test:
 	@echo ""
 	@cd $(TESTDIR); python -c "import $(PROJECT); $(PROJECT).show_versions()"
 	@echo ""
-	cd $(TESTDIR); pytest $(PYTEST_COV_ARGS) $(PROJECT)
+	cd $(TESTDIR); PYGMT_USE_EXTERNAL_DISPLAY="false" pytest $(PYTEST_COV_ARGS) $(PROJECT)
 	cp $(TESTDIR)/coverage.xml .
 	cp -r $(TESTDIR)/htmlcov .
 	rm -r $(TESTDIR)
diff --git a/doc/Makefile b/doc/Makefile
index 81b3a53f..6c4d7ab3 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -31,7 +31,8 @@ html: api
 	@echo
 	@echo "Building HTML files."
 	@echo
-	$(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html
+	# Set PYGMT_USE_EXTERNAL_DISPLAY to "false" to disable external display
+	PYGMT_USE_EXTERNAL_DISPLAY="false" $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html
 	@echo
 	@echo "Build finished. The HTML pages are in $(BUILDDIR)/html."
 
diff --git a/doc/_static/style.css b/doc/_static/style.css
index 66b20538..d8171f6f 100644
--- a/doc/_static/style.css
+++ b/doc/_static/style.css
@@ -89,6 +89,32 @@ p {
     max-width: 1000px;
 }
 
+/* Format parameters section similar to sphinx_rtd_theme v4.x.x (not a grid) */
+html.writer-html5 .rst-content dl.field-list {
+    display: initial;
+}
+
+/* Add a grey box around parameters similar to sphinx_rtd_theme v4.x.x) */
+.rst-content dl:not(.docutils) dl dt {
+    margin-bottom: 4px;
+    border: none;
+    border-left: solid 3px #ccc;
+    background: #f0f0f0;
+    color: #555;
+}
+
+.rst-content dl:not(.docutils) dt {
+    display: table;
+    margin: 6px 0;
+    font-size: 90%;
+    line-height: normal;
+    background: #e7f2fa;
+    color: #2980B9;
+    border-top: solid 3px #6ab0de;
+    padding: 6px;
+    position: relative;
+}
+
 /* Remove the padding from the Parameters table */
 .rst-content table.field-list .field-name {
     padding-left: 0px;
diff --git a/doc/api/index.rst b/doc/api/index.rst
index 3b4220b7..6460eb53 100644
--- a/doc/api/index.rst
+++ b/doc/api/index.rst
@@ -60,6 +60,13 @@ Saving and displaying the figure:
     Figure.show
     Figure.psconvert
 
+Configuring the display settings:
+
+.. autosummary::
+    :toctree: generated
+
+    set_display
+
 
 Data Processing
 ---------------
diff --git a/pygmt/__init__.py b/pygmt/__init__.py
index 033a21f6..1dc2ed79 100644
--- a/pygmt/__init__.py
+++ b/pygmt/__init__.py
@@ -25,7 +25,7 @@ from pkg_resources import get_distribution
 # Import modules to make the high-level GMT Python API
 from pygmt import datasets
 from pygmt.accessors import GMTDataArrayAccessor
-from pygmt.figure import Figure
+from pygmt.figure import Figure, set_display
 from pygmt.session_management import begin as _begin
 from pygmt.session_management import end as _end
 from pygmt.src import (
diff --git a/pygmt/figure.py b/pygmt/figure.py
index db152e0d..ef9269a5 100644
--- a/pygmt/figure.py
+++ b/pygmt/figure.py
@@ -6,9 +6,10 @@ import os
 from tempfile import TemporaryDirectory
 
 try:
-    from IPython.display import Image
-except ImportError:
-    Image = None
+    import IPython
+except KeyError:
+    IPython = None  # pylint: disable=invalid-name
+
 
 from pygmt.clib import Session
 from pygmt.exceptions import GMTError, GMTInvalidInput
@@ -25,6 +26,23 @@ from pygmt.helpers import (
 # This is needed for the sphinx-gallery scraper in pygmt/sphinx_gallery.py
 SHOWED_FIGURES = []
 
+# Configurations for figure display
+SHOW_CONFIG = {
+    "method": "external",  # Open in an external viewer by default
+}
+
+# Show figures in Jupyter notebooks if available
+if IPython:
+    get_ipython = IPython.get_ipython()  # pylint: disable=invalid-name
+    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"
+
 
 class Figure:
     """
@@ -235,62 +253,72 @@ class Figure:
         if show:
             launch_external_viewer(fname)
 
-    def show(self, dpi=300, width=500, method="static"):
+    def show(self, dpi=300, width=500, method=None):
         """
         Display a preview of the figure.
 
-        Inserts the preview in the Jupyter notebook output. You will need to
-        have IPython installed for this to work. You should have it if you are
-        using the notebook.
+        Inserts the preview in the Jupyter notebook output if available,
+        otherwise opens it in the default viewer for your operating system
+        (falls back to the default web browser).
+
+        :func:`pygmt.set_display` can select the default display method
+        (**notebook**, **external**, or **none**).
+
+        The ``method`` parameter can also override the default display method
+        for the current figure. Parameters ``dpi`` and ``width`` can be used
+        to control the resolution and dimension of the figure in the notebook.
+
+        Note: The external viewer can be disabled by setting the
+        PYGMT_USE_EXTERNAL_DISPLAY environment variable to **false**.
+        This is useful when running unit tests and building the documentation
+        in consoles without a Graphical User Interface.
 
-        If ``method='external'``, makes PDF preview instead and opens it in the
-        default viewer for your operating system (falls back to the default web
-        browser). Note that the external viewer does not block the current
-        process, so this won't work in a script.
+        Note that the external viewer does not block the current process.
 
         Parameters
         ----------
         dpi : int
-            The image resolution (dots per inch).
+            The image resolution (dots per inch) in Jupyter notebooks.
         width : int
-            Width of the figure shown in the notebook in pixels. Ignored if
-            ``method='external'``.
+            The image width (in pixels) in Jupyter notebooks.
         method : str
-            How the figure will be displayed. Options are (1) ``'static'``: PNG
-            preview (default); (2) ``'external'``: PDF preview in an external
-            program.
+            How the current figure will be displayed. Options are
 
-        Returns
-        -------
-        img : IPython.display.Image
-            Only if ``method != 'external'``.
+            - **external**: PDF preview in an external program [default]
+            - **notebook**: PNG preview [default in Jupyter notebooks]
+            - **none**: Disable image preview
         """
         # Module level variable to know which figures had their show method
         # called. Needed for the sphinx-gallery scraper.
         SHOWED_FIGURES.append(self)
 
-        if method not in ["static", "external"]:
-            raise GMTInvalidInput("Invalid show method '{}'.".format(method))
-        if method == "external":
-            pdf = self._preview(fmt="pdf", dpi=dpi, anti_alias=False, as_bytes=False)
-            launch_external_viewer(pdf)
-            img = None
-        elif method == "static":
-            png = self._preview(
-                fmt="png", dpi=dpi, anti_alias=True, as_bytes=True, transparent=True
+        # Set the display method
+        if method is None:
+            method = SHOW_CONFIG["method"]
+
+        if method not in ["external", "notebook", "none"]:
+            raise GMTInvalidInput(
+                (
+                    f"Invalid display method '{method}', "
+                    "should be either 'notebook', 'external', or 'none'."
+                )
             )
-            if Image is None:
+
+        if method in ["notebook", "none"]:
+            if IPython is None:
                 raise GMTError(
-                    " ".join(
-                        [
-                            "Cannot find IPython.",
-                            "Make sure you have it installed",
-                            "or use 'method=\"external\"' to open in an external viewer.",
-                        ]
+                    (
+                        "Notebook display is selected, but IPython is not available. "
+                        "Make sure you have IPython installed, "
+                        "or run the script in a Jupyter notebook."
                     )
                 )
-            img = Image(data=png, width=width)
-        return img
+            png = self._preview(fmt="png", dpi=dpi, anti_alias=True, as_bytes=True)
+            IPython.display.display(IPython.display.Image(data=png, width=width))
+
+        if method == "external":
+            pdf = self._preview(fmt="pdf", dpi=dpi, anti_alias=False, as_bytes=False)
+            launch_external_viewer(pdf)
 
     def shift_origin(self, xshift=None, yshift=None):
         """
@@ -396,3 +424,27 @@ class Figure:
         subplot,
         text,
     )
+
+
+def set_display(method=None):
+    """
+    Set the display method.
+
+    Parameters
+    ----------
+    method : str or None
+        The method to display an image. Choose from:
+
+        - **external**: PDF preview in an external program [default]
+        - **notebook**: PNG preview [default in Jupyter notebooks]
+        - **none**: Disable image preview
+    """
+    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' or 'none'."
+            )
+        )


Test patch:
diff --git a/pygmt/tests/test_figure.py b/pygmt/tests/test_figure.py
index 7b771b5a..213c0048 100644
--- a/pygmt/tests/test_figure.py
+++ b/pygmt/tests/test_figure.py
@@ -8,7 +8,7 @@ import os
 import numpy as np
 import numpy.testing as npt
 import pytest
-from pygmt import Figure
+from pygmt import Figure, set_display
 from pygmt.exceptions import GMTInvalidInput
 
 
@@ -142,8 +142,7 @@ def test_figure_show():
     """
     fig = Figure()
     fig.basemap(region="10/70/-300/800", projection="X3i/5i", frame="af")
-    img = fig.show(width=800)
-    assert img.width == 800
+    fig.show()
 
 
 @pytest.mark.mpl_image_compare
@@ -175,3 +174,12 @@ def test_figure_show_invalid_method():
     fig.basemap(region="10/70/-300/800", projection="X3i/5i", frame="af")
     with pytest.raises(GMTInvalidInput):
         fig.show(method="test")
+
+
+def test_figure_set_display_invalid():
+    """
+    Test to check if an error is raised when an invalid method is passed to
+    set_display.
+    """
+    with pytest.raises(GMTInvalidInput):
+        set_display(method="invalid")