Skip to content
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

Ensure plotting xarray grids with different central meridians work on some projections #560

Merged
merged 19 commits into from
Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d4282d0
Revert runfirst workaround for test_grdimage_over_dateline
weiji14 Aug 10, 2020
d6ce55e
Merge branch 'master' into test/central_meridians
weiji14 Sep 2, 2020
a302be6
Revert grdtrack example workaround in #531 crashing due to bad meridian
weiji14 Sep 2, 2020
d1f7df6
Merge branch 'master' into test/central_meridians
weiji14 Sep 4, 2020
1caaac4
Remove unneeded pytest mpl_image_compare and runfirst markers
weiji14 Sep 4, 2020
840fe6b
Merge branch 'master' into test/central_meridians
weiji14 Sep 4, 2020
0d702fd
Test different central meridians and projection system
weiji14 Sep 4, 2020
b83ddbf
Improve tests on different central meridians and standard parallels
weiji14 Sep 5, 2020
e7e51f3
Merge branch 'master' into test/central_meridians
weiji14 Sep 6, 2020
af0cc0a
Test using General Stereographic (S) instead of Transverse Mercator (T)
weiji14 Sep 6, 2020
c26d7a2
Silence pylint complaints on ALLOWED_CHARS & KEYWORD_ONLY variable names
weiji14 Sep 6, 2020
4d7d545
Mention fullname of projections in test_grdimage_central_meridians* docs
weiji14 Sep 6, 2020
ad432e5
Merge branch 'master' into test/central_meridians
weiji14 Sep 7, 2020
0134272
Add result_images folder to gitignore and make clean list
weiji14 Sep 7, 2020
f228e5c
Expect some failures on Cylindrical Equidistant (Q) plots
weiji14 Sep 8, 2020
406f847
Merge branch 'master' into test/central_meridians
weiji14 Sep 11, 2020
b3e063a
Fix doctest failures on helpers/testing.py
weiji14 Sep 11, 2020
8f14fe2
Merge branch 'master' into test/central_meridians
weiji14 Sep 11, 2020
0516b06
Initialize fig_ref and fig_test on one line instead of two
weiji14 Sep 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions pygmt/helpers/testing.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
"""
Helper functions for testing.
"""

import inspect
import os
import string

from matplotlib.testing.compare import compare_images

from ..exceptions import GMTImageComparisonFailure
from ..figure import Figure


def check_figures_equal(*, tol=0.0, result_dir="result_images"):
def check_figures_equal(*, extensions=("png",), tol=0.0, result_dir="result_images"):
"""
Decorator for test cases that generate and compare two figures.

Expand All @@ -26,6 +26,8 @@ def check_figures_equal(*, tol=0.0, result_dir="result_images"):

Parameters
----------
extensions : list
The extensions to test. Default is ["png"].
tol : float
The RMS threshold above which the test is considered failed.
result_dir : str
Expand Down Expand Up @@ -60,21 +62,30 @@ def check_figures_equal(*, tol=0.0, result_dir="result_images"):
... )
>>> shutil.rmtree(path="tmp_result_images") # cleanup folder if tests pass
"""
# pylint: disable=invalid-name
ALLOWED_CHARS = set(string.digits + string.ascii_letters + "_-[]()")
KEYWORD_ONLY = inspect.Parameter.KEYWORD_ONLY

def decorator(func):
import pytest

os.makedirs(result_dir, exist_ok=True)
old_sig = inspect.signature(func)

def wrapper(*args, **kwargs):
@pytest.mark.parametrize("ext", extensions)
def wrapper(*args, ext, request, **kwargs):
if "ext" in old_sig.parameters:
kwargs["ext"] = ext
if "request" in old_sig.parameters:
kwargs["request"] = request

file_name = "".join(c for c in request.node.name if c in ALLOWED_CHARS)
try:
fig_ref = Figure()
fig_test = Figure()
func(*args, fig_ref=fig_ref, fig_test=fig_test, **kwargs)
ref_image_path = os.path.join(
result_dir, func.__name__ + "-expected.png"
)
test_image_path = os.path.join(result_dir, func.__name__ + ".png")
ref_image_path = os.path.join(result_dir, f"{file_name}-expected.{ext}")
test_image_path = os.path.join(result_dir, f"{file_name}.{ext}")
fig_ref.savefig(ref_image_path)
fig_test.savefig(test_image_path)

Expand Down Expand Up @@ -105,9 +116,18 @@ def wrapper(*args, **kwargs):
for param in old_sig.parameters.values()
if param.name not in {"fig_test", "fig_ref"}
]
if "ext" not in old_sig.parameters:
parameters += [inspect.Parameter("ext", KEYWORD_ONLY)]
if "request" not in old_sig.parameters:
parameters += [inspect.Parameter("request", KEYWORD_ONLY)]
new_sig = old_sig.replace(parameters=parameters)
wrapper.__signature__ = new_sig

# reach a bit into pytest internals to hoist the marks from
# our wrapped function
new_marks = getattr(func, "pytestmark", []) + wrapper.pytestmark
wrapper.pytestmark = new_marks

return wrapper

return decorator
31 changes: 27 additions & 4 deletions pygmt/tests/test_grdimage.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,32 @@ def test_grdimage_over_dateline(xrgrid):


@check_figures_equal()
def test_grdimage_central_longitude(grid, fig_ref, fig_test):
@pytest.mark.parametrize("lon0", [0, 123, 180])
@pytest.mark.parametrize("proj_type", ["H", "W"])
def test_grdimage_central_meridians(grid, proj_type, lon0, fig_ref, fig_test):
"""
Test that plotting a grid centred at different longitudes/meridians work.
Test that plotting a grid with different central meridians (lon0) using
Hammer (H) and Mollweide (W) projection systems work.
"""
fig_ref.grdimage("@earth_relief_01d_g", projection="W120/15c", cmap="geo")
fig_test.grdimage(grid, projection="W120/15c", cmap="geo")
fig_ref.grdimage(
"@earth_relief_01d_g", projection=f"{proj_type}{lon0}/15c", cmap="geo"
)
fig_test.grdimage(grid, projection=f"{proj_type}{lon0}/15c", cmap="geo")
Copy link
Member Author

@weiji14 weiji14 Sep 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test checks that #515 is fixed (and it is!), specifically, that PyGMT can plot xarray grids using the Hammer (H) (and Mollweide (W)) projection system on central longitudes 0, 123 and 180deg.



@check_figures_equal()
@pytest.mark.parametrize("lat0", [0, 30])
@pytest.mark.parametrize("lon0", [0, 123, 180])
@pytest.mark.parametrize("proj_type", ["Q", "S"])
def test_grdimage_central_meridians_and_standard_parallels(
grid, proj_type, lon0, lat0, fig_ref, fig_test
):
"""
Test that plotting a grid with different central meridians (lon0) and
standard_parallels (lat0) using Cylindrical Equidistant (Q) and General
Stereographic (S) projection systems work.
"""
fig_ref.grdimage(
"@earth_relief_01d_g", projection=f"{proj_type}{lon0}/{lat0}/15c", cmap="geo"
)
fig_test.grdimage(grid, projection=f"{proj_type}{lon0}/{lat0}/15c", cmap="geo")
Copy link
Member Author

@weiji14 weiji14 Sep 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is meant to resolve @MarkWieczorek's longstanding issue at #390 (we're getting there). Specifically, we check plotting xarray grids on different central meridians (lon0) at 0, 123 and 180deg, and different standard parallels (lat0) 0, 30deg for Cylindrical Equidistant (Q) and General Stereographic (S) projections systems. These plots are compared to a baseline/reference plot using a NetCDF grid directly.

Good news: With GMT 6.1.1, plotting these grids don't crash or give a black image anymore. Also for a few lon0/lat0 combinations e.g. (Q0/0, Q0/30, S123/0, S180/0), the plots are perfect.

Bad news: Certain lon0/lat0 combinations plot in xarray, but aren't exactly the same as the NetCDF plots. There's an RMS difference of ~25 for the Cylindrical Equidistant plots Q123/0, Q123/30, Q180/0, Q180/30 (see my comment at #560 (comment) for a visual diff). The General Stereographic plots also show minor RMS differences <2 for S0/0, S0/30, S123/30, S180/30, with the diff appearing as a strip along 0deg longitude (i.e. Greenwich, see #560 (comment) for a visual diff).

I've also tested combinations of GMT_IN|GMT_IS_REFERENCE (the current setting), GMT_IN (xref #517), and GMT_IN|GMT_IS_DUPLICATE, but the results are the same. Paul's comment at GenericMappingTools/gmt#3829 (comment) does suggest that there might be outstanding issues to resolve here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking to the time to look into this. My attitude is that if you think you have done all you can with GMT 6.1.1, then you should release pygmt 0.2 as soon as possible, even if there are still some bugs that might be upstream.

pygmt 0.1.2 is completely unusable for me (for plotting global projected data with xarray grids), and as far as I can tell with my tests from pygmt/master, things appear to be ok (i.e., no kernel crashes, and the images appear to be fine without detailed inspection). Nevertheless, the problem with the last column on the right of the plot being incorrect is a clear bug and will need to resolved in the following release.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there is nothing we can do in PyGMT to fix these bugs. We could do a 0.2.0 release soon.