GenericMappingTools / pygmt

A Python interface for the Generic Mapping Tools.
https://www.pygmt.org
BSD 3-Clause "New" or "Revised" License
758 stars 220 forks source link

The dynamically generated images are not shown in the documentation #3416

Closed seisman closed 2 months ago

seisman commented 2 months ago

In https://github.com/GenericMappingTools/pygmt/pull/3379, we enabled the myst-nb extension to generate images dynamically from codes in Markdown files.

In the RTD preview (https://pygmt-dev--3379.org.readthedocs.build/en/3379/install.html#testing-your-install), the image was shown correctly, but for the documentation built by GitHub Actions, the image is not shown (see https://www.pygmt.org/dev/install.html#testing-your-install).

The difference is that, when building documentation, RTD calls sphinx-build directly, while in the "Docs" workflow, we run make -C doc clean all, which set PYGMT_USE_EXTERNAL_DISPLAY="false" before calling sphinx-build:

https://github.com/GenericMappingTools/pygmt/blob/0faf52c2410601f551e7e8661cafdd28889dd0c0/doc/Makefile#L36

The complicated thing is:

Gallery examples are executed by Sphinx-Gallery, which executes the Python codes using the compile/exec functions. Thus, the IPython kernel is not available and images are opened using external viewers by default. We have to disable it by setting PYGMT_USE_EXTERNAL_DISPLAY="false". The PyGMTScraper class keeps track of all Figure instances then call Figure.show() and save the images using Figure.savefig.

Instead, code cells in MyST Markdown files are executed by the myst-nb extension. As I understand it, it has an IPython Kernel, so the default display method is "notebook" and can't be "none".

So, the solution should be straightforward. PYGMT_USE_EXTERNAL_DISPLAY="false" should only have effects when the default method is "external".

seisman commented 2 months ago

The bug should be fixed by #3396, especially changes in 5bc139209c92efd4c8673f281372021b73941d4a.

Edit: Will cherry-pick the related changes in #3396 and open a new PR, so that #3396 can focus on the pygmt.set_display bug.