Migrate Figure.grdview tests to dvc #1154

weiji14 · 2021-04-01T04:13:44Z

Description of proposed changes

Final round of tests to migrate to DVC for the grdview plotting module. This is related to #1131, and is almost like reverting #589 actually.

Fixes #1131

Reminders

Run make format and make check to make sure the code follows the style guide.
Add tests for new features or tests that would have caught the bug that you're fixing.
Add new public functions/methods/classes to doc/api/index.rst.
Write detailed docstrings for all functions/methods.
If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

/format: automatically format and lint the code
/test-gmt-dev: run full tests on the latest GMT development version

github-actions · 2021-04-01T04:15:16Z

Summary of changed images

This is an auto-generated report of images that have changed on the DVC remote

Status	Path
added	pygmt/tests/baseline/test_grdview_drapegrid_dataarray.png
added	pygmt/tests/baseline/test_grdview_grid_dataarray.png
added	pygmt/tests/baseline/test_grdview_grid_file_with_region_subset.png
added	pygmt/tests/baseline/test_grdview_on_a_plane.png
added	pygmt/tests/baseline/test_grdview_on_a_plane_styled_with_facadepen.png
added	pygmt/tests/baseline/test_grdview_on_a_plane_with_colored_frontal_facade.png
added	pygmt/tests/baseline/test_grdview_surface_mesh_plot_styled_with_meshpen.png
added	pygmt/tests/baseline/test_grdview_surface_plot_styled_with_contourpen.png
added	pygmt/tests/baseline/test_grdview_with_cmap_for_image_plot.png
added	pygmt/tests/baseline/test_grdview_with_cmap_for_perspective_surface_plot.png
added	pygmt/tests/baseline/test_grdview_with_cmap_for_surface_monochrome_plot.png
added	pygmt/tests/baseline/test_grdview_with_perspective.png
added	pygmt/tests/baseline/test_grdview_with_perspective_and_zaxis_frame.png
added	pygmt/tests/baseline/test_grdview_with_perspective_and_zscale.png
added	pygmt/tests/baseline/test_grdview_with_perspective_and_zsize.png

None

Image diff(s)

Added images

pygmt/tests/baseline/test_grdview_drapegrid_dataarray.png

pygmt/tests/baseline/test_grdview_grid_dataarray.png

pygmt/tests/baseline/test_grdview_grid_file_with_region_subset.png

pygmt/tests/baseline/test_grdview_on_a_plane.png

pygmt/tests/baseline/test_grdview_on_a_plane_styled_with_facadepen.png

pygmt/tests/baseline/test_grdview_on_a_plane_with_colored_frontal_facade.png

pygmt/tests/baseline/test_grdview_surface_mesh_plot_styled_with_meshpen.png

pygmt/tests/baseline/test_grdview_surface_plot_styled_with_contourpen.png

pygmt/tests/baseline/test_grdview_with_cmap_for_image_plot.png

pygmt/tests/baseline/test_grdview_with_cmap_for_perspective_surface_plot.png

pygmt/tests/baseline/test_grdview_with_cmap_for_surface_monochrome_plot.png

pygmt/tests/baseline/test_grdview_with_perspective.png

pygmt/tests/baseline/test_grdview_with_perspective_and_zaxis_frame.png

pygmt/tests/baseline/test_grdview_with_perspective_and_zscale.png

pygmt/tests/baseline/test_grdview_with_perspective_and_zsize.png

Modified images

Path	Old	New

Report last updated at commit 2962bad

Related to #1131, and is almost like reverting #589.

seisman · 2021-04-01T09:14:47Z

The tests fail. Did you use GMT dev?

weiji14 · 2021-04-03T02:30:28Z

The tests fail. Did you use GMT dev?

I actually used GMT 6.1.1. Tried a fresh install and the images and their hash are the same, so not sure what's going on :/

PyGMT information:
  version: v0.3.2.dev60+gaf325094c
System information:
  python: 3.9.2 | packaged by conda-forge | (default, Feb 21 2021, 05:02:46)  [GCC 9.3.0]
  executable: /home/user/miniconda3/envs/pygmt/bin/python
  machine: Linux-5.4.0-70-generic-x86_64-with-glibc2.31
Dependency information:
  numpy: 1.20.2
  pandas: 1.2.3
  xarray: 0.17.0
  netCDF4: 1.5.6
  packaging: 20.9
  ghostscript: 9.53.3
  gmt: 6.1.1
GMT library information:
  binary dir: /home/user/miniconda3/envs/pygmt/bin
  cores: 6
  grid layout: rows
  library path: /home/user/miniconda3/envs/pygmt/lib/libgmt.so
  padding: 2
  plugin dir: /home/user/miniconda3/envs/pygmt/lib/gmt/plugins
  share dir: /home/user/miniconda3/envs/pygmt/share/gmt
  version: 6.1.1

seisman · 2021-04-03T02:57:37Z

Did you re-generate all the images? Or did you just upload the old baseline images to dvc? These grdview tests also fail for me.

seisman · 2021-04-03T03:18:06Z

It's weird. the grdview tests fail when running make test, but pass when running pytest pygmt/tests/test_grdview.py.

weiji14 · 2021-04-03T03:22:34Z

Did you re-generate all the images? Or did you just upload the old baseline images to dvc? These grdview tests also fail for me.

Yes I re-generated all the images and uploaded them. The grdview tests pass for me locally 😕

seisman · 2021-04-04T05:24:04Z

pygmt/tests/test_grdview.py

-    fig_test.grdview(grid=xrgrid)
-    return fig_ref, fig_test
+    fig = Figure()
+    fig.grdview(grid=xrgrid)


If you add frame=True to this Figure.grdview call and re-generate the baseline image, you will see a baseline image like:

It's obvious that the baseline image uses a Cartesian coordinate, but we expect it to be a geographic map, because xrgrid.gmt.gtype is 1.

Running make test, the result.png image is:

However, running pytest pygmt/tests/test_grdview.py, the result.png is:

One thing I don't get is why the @check_figures_equal test passes. Shouldn't the NetCDF grid be plotted on a geographic frame?

weiji14 · 2021-04-22T03:54:42Z

pygmt/tests/test_grdview.py

-@check_figures_equal()
-def test_grdview_with_perspective(gridfile, xrgrid):
+@pytest.mark.mpl_image_compare
+def test_grdview_with_perspective(gridfile):


This is the only grdview test failing on GMT 6.2.0rc1 at https://github.com/GenericMappingTools/pygmt/runs/2406542255?check_suite_focus=true#step:11:688. Maybe I need to regenerate the image?

New baseline On CI (failing)

GMT CLI shows that the right one is correct:

gmt grdcut @earth_relief_01d_g -R-116/-109/-47/-44 -Gabc.grd gmt grdview abc.grd -p135/15 -pdf map

Strange, ~~running make test passes locally for me~~ (Edit: actually it fails), and pytest --mpl-generate-path=baseline -k perspective pygmt/tests/test_grdview.py generates the (wrong) left image. ~~The left image seems to be plotted as a Cartesian grid instead of Geographic?~~ Nope, it's geographic, just tested with frame=True and the fancy frame shows up, so it's just the ~~rotation~~ grid shape (square vs rectangular) that's wrong for some reason.

I think it's still the geographic/cartesian issue. To reproduce it, try:

gmt grdcut @earth_relief_01d_g -R-116/-109/-47/-44 -Gabc.grd gmt grdview abc.grd -p135/15 -JQ15c -B -png right gmt grdview abc.grd -p135/15 -JX15c -B -png left

But the input to fig.grdview here is a tmpfile, not an xarray.DataArray grid, so why would there be a Geographic/Cartesian issue?

pygmt/pygmt/tests/test_grdview.py

Line 78 in 3748ecd

fig.grdview(grid=gridfile, perspective=[135, 15], frame=True)

I've tried installing fresh conda environments on both my uni and personal computer, and in both cases, the same left image appears using pytest --mpl-generate-path ..., but the pure GMT code you provided shows the correct result.

Still an issue on GMT 6.2.0rc2. I get the left image when running pytest --mpl-generate-path=baseline pygmt/tests/test_grdview.py, but make test produces the (correct) right image:

Baseline (wrong) make test (correct)

Not sure if this is an upstream GMT bug, but here are some debugging outputs:

Input gridfile's grdinfo output

/tmp/pygmt-31e64hjk.nc: Title: Produced by grdcut /tmp/pygmt-31e64hjk.nc: Command: grdcut @earth_relief_01d_g -G/tmp/pygmt-31e64hjk.nc -R-116/-109/-47/-44 /tmp/pygmt-31e64hjk.nc: Remark: Obtained by Gaussian Cartesian filtering (111.2 km fullwidth) from SRTM15+V2.1.nc [Tozer et al., 2019; http://dx.doi.org/10.1029/2019EA000658] /tmp/pygmt-31e64hjk.nc: Gridline node registration used [Geographic grid] /tmp/pygmt-31e64hjk.nc: Grid file format: nf = GMT netCDF format (32-bit float), CF-1.7 /tmp/pygmt-31e64hjk.nc: x_min: -116 x_max: -109 x_inc: 1 name: longitude n_columns: 8 /tmp/pygmt-31e64hjk.nc: y_min: -47 y_max: -44 y_inc: 1 name: latitude n_rows: 4 /tmp/pygmt-31e64hjk.nc: v_min: -3606.5 v_max: -2614.5 name: elevation (m) /tmp/pygmt-31e64hjk.nc: scale_factor: 1 add_offset: 0 /tmp/pygmt-31e64hjk.nc: format: classic

Running fig.grdview(grid=gridfile, perspective=[135, 15], frame=True, verbose="d"):

grdview [DEBUG]: Look for file -116/-109/-47/-44 in /home/username/.gmt grdview [DEBUG]: Look for file -116/-109/-47/-44 in /home/username/.gmt/cache grdview [DEBUG]: Look for file -116/-109/-47/-44 in /home/username/.gmt/server grdview [DEBUG]: Got regular w/e/s/n for region (-116/-109/-47/-44) grdview [DEBUG]: Given full path to file /tmp/pygmt-31e64hjk.nc grdview [DEBUG]: Replace file /tmp/pygmt-31e64hjk.nc with /tmp/pygmt-31e64hjk.nc grdview [DEBUG]: Given full path to file /tmp/pygmt-31e64hjk.nc grdview [DEBUG]: Given full path to file /tmp/pygmt-31e64hjk.nc grdview [DEBUG]: Found readable file /tmp/pygmt-31e64hjk.nc grdview [DEBUG]: Object ID 0 : Registered Grid File /tmp/pygmt-31e64hjk.nc as an Input resource with geometry Surface [n_objects = 1] grdview [DEBUG]: gmtapi_begin_io: Input resource access is now enabled [container] grdview [DEBUG]: gmtapi_import_grid: Passed ID = 0 and mode = 1 grdview [DEBUG]: Found readable file /tmp/pygmt-31e64hjk.nc grdview [DEBUG]: Calling nc_open on /tmp/pygmt-31e64hjk.nc, ncid = 65536, err = 0 grdview [DEBUG]: Calling nc_close on ncid 65536, err = 0 grdview [DEBUG]: Calling nc_open on /tmp/pygmt-31e64hjk.nc, ncid = 65536, err = 0 grdview [DEBUG]: Calling nc_close on ncid 65536, err = 0 grdview [DEBUG]: Call gmtgrdio_doctor_geo_increments on a geographic grid grdview [DEBUG]: Geographic input grid, longitudes span less than 360 grdview [DEBUG]: GMT_End_IO: Input resource access is now disabled grdview [DEBUG]: Reset MAP_ANNOT_OBLIQUE to anywhere grdview [DEBUG]: Projected values in meters: -3.5 3.5 -47 -44 grdview [INFORMATION]: Given view angle = 135, set MAP_FRAME_AXES = lEStZ grdview [DEBUG]: Computed automatic parameters using dimension scaling: 0.966667 grdview [DEBUG]: Auto-frame interval for axis 0 item 0: d = 2 f = 1 grdview [INFORMATION]: Auto-frame interval for x-axis (item 0): a2f1 grdview [DEBUG]: Auto-frame interval for axis 1 item 0: d = 1 f = 0.25 grdview [INFORMATION]: Auto-frame interval for y-axis (item 0): a1f0.25 grdview [INFORMATION]: Map scale is 0.000466667 km per cm or 1:46.6667. grdview [INFORMATION]: Processing shape grid grdview [DEBUG]: gmtapi_begin_io: Input resource access is now enabled [container] grdview [DEBUG]: gmtapi_import_grid: Passed ID = 0 and mode = 2 grdview [INFORMATION]: Reading grid from file /tmp/pygmt-31e64hjk.nc grdview [DEBUG]: Calling nc_open on /tmp/pygmt-31e64hjk.nc, ncid = 65536, err = 0 grdview [DEBUG]: packed z-range: [-3606.5,-2614.5] grdview [DEBUG]: Calling nc_close on ncid 65536, err = 0 grdview [DEBUG]: Geographic input grid, longitudes span less than 360 grdview [DEBUG]: Chosen boundary condition for all edges: geographic grdview [INFORMATION]: Set boundary condition for all edges: natural grdview [INFORMATION]: Set boundary condition for left edge: natural grdview [INFORMATION]: Set boundary condition for right edge: natural grdview [INFORMATION]: Set boundary condition for bottom edge: natural grdview [INFORMATION]: Set boundary condition for top edge: natural grdview [DEBUG]: GMT_End_IO: Input resource access is now disabled grdview [DEBUG]: Octant 1 (az = 14.5108) one = 1 grdview [DEBUG]: Outer loop over x doing 0:1:7 grdview [DEBUG]: Inner loop over y doing 1:1:4 grdview [INFORMATION]: Start creating PostScript plot grdview [DEBUG]: Running in PS mode modern grdview [DEBUG]: Use PS filename /home/username/.gmt/sessions/gmt_session.27871/gmt_1.ps- grdview [DEBUG]: Create hidden PS file /home/username/.gmt/sessions/gmt_session.27871/gmt_1.ps- grdview [DEBUG]: Got session name as pygmt-session and default graphics formats as pdf grdview [DEBUG]: Basemap order: Frame = above Grid = above Tick/ANot = above grdview [INFORMATION]: Do mesh plot with mesh color white grdview [DEBUG]: Current size of half-baked PS file /home/username/.gmt/sessions/gmt_session.27871/gmt_1.ps- = 24135. grdview [DEBUG]: gmtlib_garbage_collection: Destroying object: C=0 A=0 ID=0 W=Input F=Grid M=File S=Used P=563f3d35b1f0 N=/tmp/pygmt-31e64hjk.nc grdview [DEBUG]: GMTAPI_Garbage_Collection freed 1 memory objects grdview [DEBUG]: gmtlib_unregister_io: Unregistering object no 0 [n_objects = 0]

The key line is below, diff'ed with that from running grdview via GMT C:

- grdview [DEBUG]: Projected values in meters: -3.5 3.5 -47 -44 + grdview [INFORMATION]: Spherical approximation used + grdview [INFORMATION]: Central meridian not given, default to -112.5 + grdview [DEBUG]: Projected values in meters: -389183 389183 -5.22617e+06 -4.89258e+06

So you're right that PyGMT is trying to handle the input grid using Cartesian coordinates, even though a Geographic grid file (NetCDF) was passed into fig.grdview. Something wrong with the GMT C API?

maxrjones · 2021-05-25T23:23:06Z

Not sure if I am missing something - can you explain why projection is not set in the grdview tests?

weiji14 · 2021-05-25T23:42:13Z

pygmt/tests/test_grdview.py

-    fig_test.grdview(grid=xrgrid, perspective=[135, 15])
-    return fig_ref, fig_test
+    fig = Figure()
+    fig.grdview(grid=gridfile, perspective=[135, 15], frame=True)


Not sure if I am missing something - can you explain why projection is not set in the grdview tests?

Good point @meghanrjones. The projection (-J) parameter is listed as a required argument in https://docs.generic-mapping-tools.org/6.2.0rc2/grdview.html, so we probably should add it to the test? Setting projection="M6c" here does help resolve the issue at https://github.com/GenericMappingTools/pygmt/pull/1154/files#r639259330, but I'm still wondering if there's an issue with the GMT C API's auto projection setting.

There shouldn't be an auto projection setting unless it was set previously in a modern mode session 😰 I get an appropriate error when I run an equivalent gmt command, so it is odd that it passes without complaint for PyGMT. I can look into this unless you want to.

$gmt grdcut @earth_relief_01d_g -R-116/-109/-47/-44 -Gtmp.nc $gmt grdview tmp.nc -p135/15 grdview [WARNING]: The -p option works best in consort with -J (and -R or a grid) grdview [ERROR]: Must specify a map projection with the -J option

I think the default projection is Q15c+ for geographic data and X15c for cartesian data.

Ah, thanks for the clarification, feel free to look into it (I got a lunch seminar to go to soon). I get the same error running grdview on the command line, so it must be something with PyGMT's global session that set a projection before this test ran.

I think the default projection is Q15c+ for geographic data and X15c for cartesian data.

Is this a GMT default or a PyGMT default?

Some testing shows that it's a modern mode GMT default. Always more to learn it seems.

Ok, I've added projection="Q15c+" to the grdview test at 2962bad so it should be ok now. The auto/default projection issue can be discussed another time 🙂

Final round of tests to migrate to DVC for the `grdview` plotting module. This is related to GenericMappingTools#1131, and is almost like reverting GenericMappingTools#589 actually. * Migrate test_grdview baseline images to dvc * Update fig.grdview baseline images for GMT 6.2.0rc1 * Plot fancy frame for test_grdview_with_perspective * Update fig.grdview baseline images for GMT 6.2.0rc2 * Use projection="Q15c+" for test_grdview_with_perspective

weiji14 added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog labels Apr 1, 2021

weiji14 added this to the 0.4.0 milestone Apr 1, 2021

weiji14 self-assigned this Apr 1, 2021

Migrate test_grdview baseline images to dvc

53ddba4

Related to #1131, and is almost like reverting #589.

weiji14 force-pushed the dvc-migrate/grdview branch from dbca95d to 53ddba4 Compare April 1, 2021 04:19

vercel bot temporarily deployed to Preview April 1, 2021 04:20 Inactive

michaelgrund mentioned this pull request Apr 1, 2021

Migrate tests to use dvc-tracked baseline images #1131

Closed

28 tasks

Merge branch 'master' into dvc-migrate/grdview

af32509

weiji14 changed the title ~~Migrate test_grdview baseline images to dvc~~ Migrate Figure.grdview baseline images to dvc Apr 3, 2021

weiji14 changed the title ~~Migrate Figure.grdview baseline images to dvc~~ Migrate Figure.grdview tests to dvc Apr 3, 2021

vercel bot temporarily deployed to Preview April 3, 2021 02:27 Inactive

seisman reviewed Apr 4, 2021

View reviewed changes

seisman mentioned this pull request Apr 4, 2021

Geographic xarray.DataArray grid is plotted as Cartesian grid #1172

Closed

weiji14 added 2 commits April 22, 2021 15:29

Merge branch 'master' into dvc-migrate/grdview

4f7b144

Update fig.grdview baseline images for GMT 6.2.0rc1

00067a0

vercel bot temporarily deployed to Preview April 22, 2021 03:35 Inactive

weiji14 commented Apr 22, 2021

View reviewed changes

weiji14 added 2 commits April 23, 2021 12:11

Merge branch 'master' into dvc-migrate/grdview

76de3e0

Plot fancy frame for test_grdview_with_perspective

3748ecd

vercel bot temporarily deployed to Preview April 23, 2021 00:13 Inactive

weiji14 mentioned this pull request Apr 23, 2021

Bump to GMT 6.2.0rc1 #1217

Closed

3 tasks

Merge branch 'master' into dvc-migrate/grdview

e083241

vercel bot temporarily deployed to Preview May 21, 2021 03:29 Inactive

weiji14 added 2 commits May 25, 2021 23:19

Merge branch 'master' into dvc-migrate/grdview

e7e732a

Update fig.grdview baseline images for GMT 6.2.0rc2

0832f16

vercel bot temporarily deployed to Preview May 25, 2021 11:28 Inactive

Merge branch 'master' into dvc-migrate/grdview

a91762f

vercel bot temporarily deployed to Preview May 25, 2021 20:02 Inactive

weiji14 marked this pull request as ready for review May 25, 2021 20:02

weiji14 commented May 25, 2021

View reviewed changes

Use projection="Q15c+" for test_grdview_with_perspective

2962bad

vercel bot temporarily deployed to Preview May 26, 2021 01:12 Inactive

seisman approved these changes May 26, 2021

View reviewed changes

weiji14 merged commit 45887f1 into master May 26, 2021

weiji14 deleted the dvc-migrate/grdview branch May 26, 2021 04:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Figure.grdview tests to dvc #1154

Migrate Figure.grdview tests to dvc #1154

weiji14 commented Apr 1, 2021 •

edited

Loading

github-actions bot commented Apr 1, 2021 •

edited

Loading

Added images

Modified images

seisman commented Apr 1, 2021

weiji14 commented Apr 3, 2021

seisman commented Apr 3, 2021

seisman commented Apr 3, 2021

weiji14 commented Apr 3, 2021

seisman Apr 4, 2021

seisman Apr 4, 2021

weiji14 Apr 4, 2021

weiji14 Apr 22, 2021 •

edited

Loading

seisman Apr 22, 2021

weiji14 Apr 22, 2021 •

edited

Loading

seisman Apr 23, 2021

weiji14 Apr 23, 2021 •

edited

Loading

weiji14 May 25, 2021 •

edited

Loading

maxrjones commented May 25, 2021

weiji14 May 25, 2021

maxrjones May 25, 2021

seisman May 25, 2021

weiji14 May 25, 2021

maxrjones May 25, 2021

maxrjones May 26, 2021

weiji14 May 26, 2021

Migrate Figure.grdview tests to dvc #1154

Migrate Figure.grdview tests to dvc #1154

Conversation

weiji14 commented Apr 1, 2021 • edited Loading

github-actions bot commented Apr 1, 2021 • edited Loading

Summary of changed images

Image diff(s)

Added images

Modified images

seisman commented Apr 1, 2021

weiji14 commented Apr 3, 2021

seisman commented Apr 3, 2021

seisman commented Apr 3, 2021

weiji14 commented Apr 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiji14 Apr 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiji14 Apr 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiji14 Apr 23, 2021 • edited Loading

Choose a reason for hiding this comment

weiji14 May 25, 2021 • edited Loading

Choose a reason for hiding this comment

maxrjones commented May 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiji14 commented Apr 1, 2021 •

edited

Loading

github-actions bot commented Apr 1, 2021 •

edited

Loading

weiji14 Apr 22, 2021 •

edited

Loading

weiji14 Apr 22, 2021 •

edited

Loading

weiji14 Apr 23, 2021 •

edited

Loading

weiji14 May 25, 2021 •

edited

Loading