-
Notifications
You must be signed in to change notification settings - Fork 228
Properly allow for either pixel or gridline registered grids #476
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 32 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 |
---|---|---|
|
@@ -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, 91, 1) | ||
x = np.sin(np.deg2rad(longitude)) | ||
y = np.linspace(start=0, stop=1, num=180) | ||
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,18 @@ def test_grdimage_fails(): | |
fig = Figure() | ||
with pytest.raises(GMTInvalidInput): | ||
fig.grdimage(np.arange(20).reshape((4, 5))) | ||
|
||
|
||
@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 |
||
""" | ||
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? |
||
""" | ||
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/1i", V="d") | ||
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.
It seems like we practically always create a memory copy here because the numpy view on a flipped grid
grid[::-1]
is never (or hardly ever?) c_contiguous. I don't know if there's a way around this, but if we need to switch to useGMT_IS_DUPLICATE
for stability in #517, wouldn't that mean we create 2 memory copies?!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.
I'll try using
xarray.DataArray.copy
at 5a685b2 to force a deep copy (that should return a c_contiguous array) before it enters theas_c_contiguous
function (which may or may not return a numpy copy). It seems to give a more consistent black image, rather than the reddish-brown one 😅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.
Reverted 5a685b2 at 60c791d, will stick to just
grid[::-1].values
for now (i.e. flipping the grid left right in xarray rather than in numpy).