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

Wrap grdview #330

Merged
merged 15 commits into from
Mar 26, 2020
Merged

Wrap grdview #330

merged 15 commits into from
Mar 26, 2020

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Sep 29, 2019

Description of proposed changes

Wrapping the grdview function!

grdview perspective surface plot

Also taking a cue from the Julia wrapper/implementation at https://www.generic-mapping-tools.org/GMT.jl/latest/#GMT.grdview and GenericMappingTools/GMT.jl#218.

Parameters/Aliases to wrap:

  • Jz = zscale
  • JZ = zsize
  • p = perspective
  • B = frame (or axis in GMT.jl)
  • C = cmap
  • G = drapegrid (or drapefile in GMT.jl)
  • I = intensity
  • N = plane
  • P = portrait
  • Q = surftype
  • S = smooth
  • T = no_interp
  • W (called 'pens' in GMT.jl)
    • Wc = contourpen
    • Wm = meshpen
    • Wf = facadepen
  • U = stamp
    ...

Fixes #328

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.

Initial commit for wrapping the grdview function raised at #328, to be implemented under base_plotting.py alongside the other mapping related stuff. Original GMT `grdview` documentation can be found at https://docs.generic-mapping-tools.org/latest/grdview.html.

Storing sample test cases under test_grdview.py. Current implementation takes in a reliefgrid either as a filename or xarray.DataArray. Aliased the 'required' arguments 'Jz' to 'zscale' and 'JZ' to 'zsize' (similar to GMT julia wrappper). Also aliased 'p' to 'perspective' (as is standard in upstream GMT) so that we can have some 3D test plots to compare against, instead of just some flat (birds-eye view) maps.
Giving our grdview wrapper the ability to color plots! The cmap (C) parameter (which stands for colormap) should be straightforward, while the surftype (Q) parameter is a little bit more complicated. Some more tests added to check that we can indeed produce colored perspective-view plots.
Included test for setting a plain z-axis plane at a fixed height, and a test for colouring the frontal facade too.
Included test to check that plotting a vertical z-axis frame works.
Plus add test examples ensuring the different pens work.
@weiji14 weiji14 added this to the 0.1.0 milestone Oct 4, 2019
Included one test for a valid (xarray.DataArray) drapegrid input, and one test for an invalid input. Also using contextlib.ExitStack() as we have more than one file_context (one for reliefgrid, one for drapegrid) to enter and exit properly (see https://stackoverflow.com/questions/3024925/create-a-with-block-on-several-context-managers).
@weiji14 weiji14 self-assigned this Oct 9, 2019
@weiji14 weiji14 marked this pull request as ready for review October 9, 2019 10:28
@weiji14 weiji14 requested a review from a team October 9, 2019 10:29
Refresh baseline plots to use SRTM15+V2 grids as per #350, and also some fixed some filenames after being caught by pytest's matplotlib image comparison (thanks to #352!).
Wrap grdview docstrings to 79 characters as per #384 and make alias formatting nicer after #383.
@weiji14
Copy link
Member Author

weiji14 commented Dec 6, 2019

Ready for review. I've updated the docstrings to conform to the new 79 char length standard and make it look nicer with napolean sphinx. Rendered docs at https://pygmt-38g8s97im.now.sh/api/generated/pygmt.Figure.grdview.html#pygmt.Figure.grdview.

weiji14 added 2 commits March 26, 2020 17:21
Refresh baseline plots to use SRTM15+V2.1 grids as per #401. Ran `pytest --mpl-generate-path=baseline pygmt/tests/test_grdview.py` to generate the png images.
Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

Many thanks for keeping this going! Looks good to me. Just one minor suggestion to be consistent with the other methods.

pygmt/base_plotting.py Outdated Show resolved Hide resolved
@weiji14 weiji14 merged commit 088b0d1 into GenericMappingTools:master Mar 26, 2020
@weiji14 weiji14 mentioned this pull request May 31, 2020
@weiji14 weiji14 mentioned this pull request Jun 24, 2020
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrapper for grdview
2 participants