GenericMappingTools / pygmt

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

Use vendored check_figures_equal decorator function #579

Open weiji14 opened 4 years ago

weiji14 commented 4 years ago

Description of the desired feature

This is a medium to long term maintenance issue with the check_figures_equal function at pygmt/helpers/testing.py, which is pretty much an exact copy of matplotlib's check_figures_equal function. Ideally, we would just use a vendored function from matplotlib or pytest-mpl instead of writing our own.

Yes, which was partly why I opened up the issue at https://github.com/matplotlib/pytest-mpl/issues/94, to get all of that pytest-mpl goodness (e.g. not having a hardcoded result_dir). I'll try to make a Pull Request to pytest-mpl for that, so we can just use a proper @pytest.mark.mpl_check_equal decorator in the future (will open a new issue after this one is merged). For now though, since we don't have many tests using check_figures_equal yet, we can probably just leave it like so.

_Originally posted by @weiji14 in https://github.com/GenericMappingTools/pygmt/pull/555#discussion_r483292810_

Are you willing to help implement and maintain this feature? Yes, PR in the works at https://github.com/matplotlib/pytest-mpl/pull/95.

willschlitzer commented 3 years ago

@weiji14 It looks like Option 2 at over your pytest-mpl pull request is the same syntax as what we currently use for @check_figures_equal. Assuming your pull request is merged, would the only real change for PyGMT tests be different import statements to use the decorator?

seisman commented 3 days ago

Currently, check_figures_equal is only used in the test_grdimage.py file (https://github.com/search?q=repo%3AGenericMappingTools%2Fpygmt%20check_figures_equal&type=code). I'm wondering if we can fully get rid of check_figures_equal and always use mpl_image_compare instead.

weiji14 commented 3 days ago

I would probably keep check_figures_equal until we fix https://github.com/GenericMappingTools/pygmt/issues/390, and can be confident that plotting from a NetCDF file vs an xarray.DataArray is always the same.