-
Notifications
You must be signed in to change notification settings - Fork 219
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
Improve Figure.show for displaying previews in Jupyter notebooks and external viewers #529
Conversation
@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 |
cc @MarkWieczorek who made some comments in #366 (and this PR should ideally close it). Also might want to check out how 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). |
d7e0fbc
to
f68554c
Compare
… external viewers `Figure.show()` now is able to detect the current running environment, and display previews in Jupyter notebooks, or open previews using external viewers. The old `Figure.show(method="external")` way is no longer supported. So it's a breaking change. This PR also provides `pygmt.set_display()` to control the display mode and preview image resolution. Setting environmental variable `PYGMT_DISABLE_EXTERNAL_DISPLAY` to `true` can disable external viewers, mostly for running tests and building the documentations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
Co-authored-by: Meghan Jones <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a great improvement to me! It worked well in a few different environments on MacOS and Windows.
Regarding whether figure.show() should return an image, I think that could be a separate feature request and that this PR is a good baseline for any subsequent improvements before v0.4.0.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you can add a test for invalid input to set_display that would be good. Otherwise this looks good to me. I also do not know a way to add more unit tests, but expect any breaking changes would be noticed very soon. Please wait for @weiji14's approval before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let's get this in then! Will see if anyone else wants to comment, otherwise merge it tomorrow.
…ingTools#952) This PR improves the `launch_external_viewer` function to open images using the associated application (just like double-click the images) on Windows. The implemention is done by calling the [`os.startfile`](https://docs.python.org/3/library/os.html#os.startfile) function. `os.startfile` is only available on Windows, so need to disable the pylint error `no-member`. The code was originall written in GenericMappingTools#269 by @leouieda, re-used in GenericMappingTools#529.
…external viewers (GenericMappingTools#529) Figure.show() now is able to detect the current running environment, and display previews in Jupyter notebooks, or open previews using external viewers. This PR also provides `pygmt.set_display()` to change the display method globally. Setting environmental variable `PYGMT_USE_EXTERNAL_DISPLAY ` to `false` can disable external viewers, mostly for running tests and building the documentations. Figure.show() no longer returns the Image object. Co-authored-by: Wei Ji <[email protected]> Co-authored-by: Meghan Jones <[email protected]>
Problem
The problem was first raised by @leouieda in issue #269. In summary, there are too many inconsistent ways for displaying PyGMT images:
Figure.show()
inserts a PNG image in a Jupyter notebookFigure.show(method="external")
opens a preview in an external PDF viewer or the browser.Figure.savefig(..., show=True)
save the figure and open it on an external viewerThe main problem is,
Figure.show()
only works in Jupyter notebooks, and when running PyGMT in scripts, we have to useFigure.show(method="external")
(see a related issue #366). In the PyGMT documentation, we useFigure.show()
in the tutorials and gallery examples, and these examples won't work if users copy and save the codes in Python files. It definitely cause a lot of confusion, that's why we have to add some notes at the beginning of each tutorial (#806).The Design in PR #528
In #269, @leouieda also proposed a new design of the PyGMT display mechanism.
The core idea of the proposal in #269 is to have a uniform
Figure.show()
function to open images using external viewers or insert images in Jupyter notebooks. #528 is a draft implementation by @leouieda. It was started a year ago but never finished.The New Design in this PR
This PR implements a new design for the display mechanism. The new design follows the discussions in #269, but is slightly different from #269 and #528 in details.
In the new design of this PR, PyGMT tries to detect the current running environment (is it in a normal terminal or a Jupyter notebook), and show images in a Jupyter notebook if possible. Otherwise, it opens images using external viewers. So,
Figure.show()
will work in both Jupyter notebooks and Python scripts in a consistent way.Here are the default behaviors for some common use cases:
python display.py
ipython display.py
.jupyter qtconsole
jupyter notebook
orjupyter lab
jupyter console
Figure.show()
multiple times, ideally, it should display the 1st figure, halts the Python process until the viewer is closed, and then display the 2nd figure. As I understand it, it's impossible.When building documentation, it's also possible to disable external display using any one of the two ways:
export PYGMT_USE_EXTERNAL_DISPLAY=false
pygmt.set_display(method="notebook")
Summarized features in this PR
Figure.show
) to preview images in both Jupyter notebooks and external viewersPYGMT_USE_EXTERNAL_DISPLAY
tofalse
(useful for building documentation)pygmt.set_display(method=None, dpi=None, width=None)
to set the display method ("external" or "notebook"), and image dpi (default to 300) and width (default to 500) in Jupyter notebooks, globally. It affects all the subsequent figures.Figure.show(method=None, dpi=None, width=None)
can set the display method ("external" or "notebook"), and image dpi and width in Jupyter notebooks, locally. It only affects the current figure.Testing this PR
As listed in the "TODO" section, I don't write add unit tests for the new feature yet. So we need your help to test this PR manually. Here I provide a series of steps you can follow to help test this PR.
Testing the choice of notebook display or external viewer
test1.py
), and run it usingpython test1.py
, oripython test1.py
. It will open the image in an external viewer, but you may encounter the issue in Fail to open images in external viewer programs if running PyGMT in a Python script #1061ipython notebook
,jupyter notebook
orjupyter lab
). You should see an image inserted in the notebook.Testing the display in notebook
In a Jupyter notebook, run:
Putting the Figure instance
fig
in the last cell and run, it will also display an image, similar toFigure.show()
Testing changing display method globally or locally
Images will be inserted into the notebook by default. As mentioned above, there are two ways to change the display method:
pygmt.set_display(method="xxx")
Figure.show(method="xxx")
The below script can be used to test if the two methods work as expected. You could run it in a Jupyter notebook, in a Python file, or in a Python/IPython interpreter. The comments in the script indicate the expected behavior in a Jupyter notebook:
Testing change dpi and width
SKIP THIS TEST! THE FEATURE IS TEMPORARILY REMOVED FROM THIS PR!
TODO
Fixes #269. Fixes #366.
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.