-
Notifications
You must be signed in to change notification settings - Fork 225
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
Properly allow for either pixel or gridline registered grids #476
Changes from all commits
ebd3e00
bb7afdc
08edf6f
8ce734d
47b5b68
522ceba
d203cd5
90b22a4
6d4ece4
b252b9d
93881c5
572b499
0812c50
5cb2cee
16d9240
046a5e3
3215b0c
88ceba2
bc2c2f0
bff3254
1fc91f8
6892e4b
d8fd6b5
0e593d8
00a46ca
0f84f04
5c5be94
22dfe74
53dc9c9
0a73114
fe0fbc5
5a0c639
5a685b2
40bc7f9
d0be1c5
46c8471
3499047
bf03e5b
60c791d
c411a95
217a7d7
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 |
---|---|---|
|
@@ -50,7 +50,7 @@ def dataarray_to_matrix(grid): | |
>>> grid = load_earth_relief(resolution='01d') | ||
>>> matrix, region, inc = dataarray_to_matrix(grid) | ||
>>> print(region) | ||
[-179.5, 179.5, -89.5, 89.5] | ||
[-180.0, 180.0, -90.0, 90.0] | ||
>>> print(inc) | ||
[1.0, 1.0] | ||
>>> type(matrix) | ||
|
@@ -67,7 +67,7 @@ def dataarray_to_matrix(grid): | |
>>> print(matrix.shape) | ||
(31, 71) | ||
>>> print(region) | ||
[-149.5, -79.5, -79.5, -49.5] | ||
[-150.0, -79.0, -80.0, -49.0] | ||
>>> print(inc) | ||
[1.0, 1.0] | ||
>>> # but not if only taking every other grid point. | ||
|
@@ -77,7 +77,7 @@ def dataarray_to_matrix(grid): | |
>>> print(matrix.shape) | ||
(16, 36) | ||
>>> print(region) | ||
[-149.5, -79.5, -79.5, -49.5] | ||
[-150.5, -78.5, -80.5, -48.5] | ||
>>> print(inc) | ||
[2.0, 2.0] | ||
|
||
|
@@ -102,14 +102,19 @@ def dataarray_to_matrix(grid): | |
dim | ||
) | ||
) | ||
region.extend([coord.min(), coord.max()]) | ||
region.extend( | ||
[ | ||
coord.min() - coord_inc / 2 * grid.gmt.registration, | ||
coord.max() + coord_inc / 2 * grid.gmt.registration, | ||
] | ||
) | ||
inc.append(coord_inc) | ||
|
||
if any([i < 0 for i in inc]): # Sort grid when there are negative increments | ||
if any(i < 0 for i in inc): # Sort grid when there are negative increments | ||
inc = [abs(i) for i in inc] | ||
grid = grid.sortby(variables=list(grid.dims), ascending=True) | ||
|
||
matrix = as_c_contiguous(grid.values[::-1]) | ||
matrix = as_c_contiguous(grid[::-1].values) | ||
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. It seems like we practically always create a memory copy here because the numpy view on a flipped grid 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. I'll try using 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. |
||
return matrix, region, inc | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
Test Figure.grdimage | ||
""" | ||
import numpy as np | ||
import xarray as xr | ||
import pytest | ||
|
||
from .. import Figure | ||
|
@@ -15,6 +16,27 @@ def fixture_grid(): | |
return load_earth_relief(registration="gridline") | ||
|
||
|
||
@pytest.fixture(scope="module", name="xrgrid") | ||
def fixture_xrgrid(): | ||
""" | ||
Create a sample xarray.DataArray grid for testing | ||
""" | ||
longitude = np.arange(0, 360, 1) | ||
latitude = np.arange(-89, 90, 1) | ||
x = np.sin(np.deg2rad(longitude)) | ||
y = np.linspace(start=0, stop=1, num=179) | ||
data = y[:, np.newaxis] * x | ||
|
||
return xr.DataArray( | ||
data, | ||
coords=[ | ||
("latitude", latitude, {"units": "degrees_north"}), | ||
("longitude", longitude, {"units": "degrees_east"}), | ||
], | ||
attrs={"actual_range": [-1, 1]}, | ||
) | ||
|
||
|
||
@pytest.mark.mpl_image_compare | ||
def test_grdimage(grid): | ||
"Plot an image using an xarray grid" | ||
|
@@ -51,3 +73,23 @@ def test_grdimage_fails(): | |
fig = Figure() | ||
with pytest.raises(GMTInvalidInput): | ||
fig.grdimage(np.arange(20).reshape((4, 5))) | ||
|
||
|
||
# This test needs to run first before the other tests (on Linux at least) so | ||
# that a black image isn't plotted due to an `inf` value when resampling. | ||
# See also https://github.com/GenericMappingTools/pygmt/pull/476 | ||
@pytest.mark.runfirst | ||
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. Also add a brief comment here, explain why we need "runfirst". |
||
@pytest.mark.mpl_image_compare | ||
def test_grdimage_over_dateline(xrgrid): | ||
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. I'm starting to think that this test is exposing some flaky tests in our test suite (see also #327 (comment)). The test passes pretty much all the time (?) when I just run I probably went down the wrong rabbit hole, but I've tried pretty much all of the pytest plugins to test for flakiness including pytest-xdist and pytest-flakefinder, and they pretty much agree that it's not
Comment on lines
+81
to
+83
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. Running |
||
""" | ||
Ensure no gaps are plotted over the 180 degree international dateline. | ||
Specifically checking that `xrgrid.gmt.gtype = 1` sets `GMT_GRID_IS_GEO`, | ||
and that `xrgrid.gmt.registration = 0` sets `GMT_GRID_NODE_REG`. Note that | ||
there would be a gap over the dateline if a pixel registered grid is used. | ||
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. Add a link to issue #375? |
||
See also https://github.com/GenericMappingTools/pygmt/issues/375. | ||
""" | ||
fig = Figure() | ||
assert xrgrid.gmt.registration == 0 # gridline registration | ||
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. 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. 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. Nope, still get the all black image :/ 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. I ran the tests 10 times, and always get the expected figure, but I'm using macOS. 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. Can you add 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. Hold on, that error message was from one of the successful runs. It seems to work fine in ipython but fails when I run it using 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. Ok, got the failing debug info (not sure why it's not printed in the CI). The main difference seems to be an
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. Yes, I saw the similar inf when debugging the 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. It could be another potential GMT bug, which is usually difficult to debugging from the PyGMT side. 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. FYI, this is the output using
|
||
xrgrid.gmt.gtype = 1 # geographic coordinate system | ||
fig.grdimage(grid=xrgrid, region="g", projection="A0/0/1c", V="i") | ||
return fig |
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.
Please add some comments here, explain why we need two pytest.