-
Notifications
You must be signed in to change notification settings - Fork 234
Make IPython partially optional on CI to increase test coverage of figure.py #1496
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
Changes from all commits
4c88e0d
e5fddaa
ef11d94
32e5483
7273282
8512189
939171c
de6727a
59ba1c9
cae6190
edd1aa8
8ba65ea
a9a5796
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,17 @@ | |
| """ | ||
| import os | ||
|
|
||
| try: | ||
| import IPython | ||
| except ModuleNotFoundError: | ||
| IPython = None # pylint: disable=invalid-name | ||
|
|
||
|
|
||
| import numpy as np | ||
| import numpy.testing as npt | ||
| import pytest | ||
| from pygmt import Figure, set_display | ||
| from pygmt.exceptions import GMTInvalidInput | ||
| from pygmt.exceptions import GMTError, GMTInvalidInput | ||
| from pygmt.helpers import GMTTempFile | ||
|
|
||
|
|
||
|
|
@@ -52,6 +58,23 @@ def test_figure_region_country_codes(): | |
| npt.assert_allclose(fig.region, np.array([0.0, 360.0, -90.0, 90.0])) | ||
|
|
||
|
|
||
| def test_figure_repr(): | ||
| """ | ||
| Make sure that figure output's PNG and HTML printable representations look | ||
| ok. | ||
| """ | ||
| fig = Figure() | ||
| fig.basemap(region=[0, 1, 2, 3], frame=True) | ||
| # Check that correct PNG 8-byte file header is produced | ||
| # https://en.wikipedia.org/wiki/Portable_Network_Graphics#File_header | ||
| repr_png = fig._repr_png_() # pylint: disable=protected-access | ||
| assert repr_png.hex().startswith("89504e470d0a1a0a") | ||
| # Check that correct HTML image tags are produced | ||
| repr_html = fig._repr_html_() # pylint: disable=protected-access | ||
| assert repr_html.startswith('<img src="data:image/png;base64,') | ||
| assert repr_html.endswith('" width="500px">') | ||
|
|
||
|
|
||
| def test_figure_savefig_exists(): | ||
| """ | ||
| Make sure the saved figure has the right name. | ||
|
|
@@ -159,6 +182,7 @@ def mock_psconvert(*args, **kwargs): # pylint: disable=unused-argument | |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.skipif(IPython is None, reason="run when IPython is installed") | ||
| def test_figure_show(): | ||
| """ | ||
| Test that show creates the correct file name and deletes the temp dir. | ||
|
|
@@ -199,6 +223,27 @@ def test_figure_show_invalid_method(): | |
| fig.show(method="test") | ||
|
|
||
|
|
||
| @pytest.mark.skipif(IPython is not None, 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 is not installed. | ||
| """ | ||
| fig = Figure() | ||
| fig.basemap(region=[0, 1, 2, 3], frame=True) | ||
| with pytest.raises(GMTError): | ||
| fig.show(method="notebook") | ||
|
|
||
|
|
||
| def test_figure_display_external(): | ||
| """ | ||
| Test to check that a figure can be displayed in an external window. | ||
| """ | ||
| fig = Figure() | ||
| fig.basemap(region=[0, 3, 6, 9], projection="X1c", frame=True) | ||
| fig.show(method="external") | ||
|
Comment on lines
+238
to
+244
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The CI can't open external viewers. Why does this test work? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the console won't show a window pop-up, but I think the code will still execute fine somehow. Looking at the codecov diff at https://app.codecov.io/gh/GenericMappingTools/pygmt/compare/1496/changes#D2L237, the |
||
|
|
||
|
|
||
| def test_figure_set_display_invalid(): | ||
| """ | ||
| Test to check if an error is raised when an invalid method is passed to | ||
|
|
||

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.
Is
_repr_png_and_repr_html_useful? I have no idea how these functions can be used.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.
Good question. They are used by
IPython.display.display_pngandIPython.display.display_htmlrespectively, i.e. for display in Jupyter notebooks.Technically I should have written these unit test using
IPython.display.display_png(fig)andIPython.display.display_html(fig)instead of testing these non-public functions directly, but it's hard to test a pop-up figure. So I've settled for testing the__repr_png__and__repr_html__instead (which works with or without IPython installed). Con with this approach is that I need to dopylint: disable=protected-access🙃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.
PyGMT already provides the
Figure.show()function for display in Jupyter notebooks. Removing_repr_png_()and_repr_html_()doesn't affect theFigure.show()function. Do you think we should remove these two functions?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.
It might not affect
Figure.show(), but I think we need to keep it. Having__repr_png__()and__repr_html__means that putting justfigat the last line of a Jupyter notebook cell will display the plot. E.g. like this:This works in Jupyter/IPython because of a
IPython.display.display_html(fig)call. Similarly, if you put adf(pandas.DataFrame) on the last line of a cell, a nicely formatted html table will be printed because Jupyter/IPython doesIPython.display.display_html(df)(see the__repr_html__at https://github.com/pandas-dev/pandas/blob/73c68257545b5f8530b7044f56647bd2db92e2ba/pandas/core/frame.py#L1007-L1049).