Make IPython partially optional on CI to increase test coverage of figure.py #1496
Conversation
weiji14
changed the title
weiji14
changed the title
| 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 |
There was a problem hiding this comment.
Is _repr_png_ and _repr_html_ useful? I have no idea how these functions can be used.
There was a problem hiding this comment.
Good question. They are used by IPython.display.display_png and IPython.display.display_html respectively, i.e. for display in Jupyter notebooks.
Technically I should have written these unit test using IPython.display.display_png(fig) and IPython.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 do pylint: disable=protected-access 🙃
There was a problem hiding this comment.
They are used by
IPython.display.display_pngandIPython.display.display_htmlrespectively, i.e. for display in Jupyter notebooks.
PyGMT already provides the Figure.show() function for display in Jupyter notebooks. Removing _repr_png_() and _repr_html_() doesn't affect the Figure.show() function. Do you think we should remove these two functions?
There was a problem hiding this comment.
It might not affect Figure.show(), but I think we need to keep it. Having __repr_png__() and __repr_html__ means that putting just fig at 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 a df (pandas.DataFrame) on the last line of a cell, a nicely formatted html table will be printed because Jupyter/IPython does IPython.display.display_html(df) (see the __repr_html__ at https://github.com/pandas-dev/pandas/blob/73c68257545b5f8530b7044f56647bd2db92e2ba/pandas/core/frame.py#L1007-L1049).
| 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") |
There was a problem hiding this comment.
The CI can't open external viewers. Why does this test work?
There was a problem hiding this comment.
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 launch_external_viewer function is being called up to the time.sleep(0.5) line, so it seems that Linux/macOS/Windows launchers (xdg-open/open/not-sure-what's-on windows) are available on CI?
seisman
added
the
final review call
weiji14
removed
the
final review call
…gure.py (GenericMappingTools#1496) Makes IPython an optional dependency in Continuous Integration for the minimum dependency stream (Python 3.7/NumPy 1.18), so that uncovered if-branches in `figure.py` are covered. Tests which require IPython have a pytest.importorskip line added so that the test suite doesn't break. * Install IPython as an optional dependency only for Python 3.9 CI tests * Add pytest.importorskip ipython to test_figure_show * Add pytest.importorskip ipython and sphinx_gallery to test_pygmtscrapper * Skip running the fig.inset doctest's fig.show line * Add test for fig.show using notebook method when IPython isn't installed * Add tests for figure's PNG and HTML printable representations * Provide reason for skipping test_figure_show and test_pygmtscrapper * Silence pylint protected-access warning on accessing PNG and HTML repr * Try test figure display external method * Move pytest.importorskip IPython outside of test_pygmtscrapper Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Description of proposed changes
The
pygmt/figure.pyfile has the smallest code coverage in the PyGMT project, with 15 lines uncovered. This is mostly due to if-branches that only get triggered whenIPythonisn't installed.This PR makes
IPythonan optional dependency in Continuous Integration for the minimum dependency stream (Python 3.7/NumPy 1.18), so that those uncovered if-branches are covered. Tests which requireIPythonwill have apytest.importorskipline added so that the test suite doesn't break.See coverage diff at https://app.codecov.io/gh/GenericMappingTools/pygmt/compare/1496/changes. Project test coverage increased by 0.96% to 98.25%!
References:
Addresses #1247 (comment) and patches #529 with more code coverage.
Reminders
make formatandmake checkto make sure the code follows the style guide.doc/api/index.rst.Slash Commands
You can write slash commands (
/command) in the first line of a comment to performspecific operations. Supported slash commands are:
/format: automatically format and lint the code/test-gmt-dev: run full tests on the latest GMT development version