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

Add gallery example for grdview #502

Merged
merged 6 commits into from
Jul 2, 2020
Merged

Add gallery example for grdview #502

merged 6 commits into from
Jul 2, 2020

Conversation

liamtoney
Copy link
Member

Description of proposed changes

Add an example use of grdview() to the gallery. Serves as an example for #433, as well as a refresher for me. 😝

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.

@liamtoney liamtoney added the scipy-sprint Things to work on during the SciPy sprint! label Jul 1, 2020
@vercel vercel bot temporarily deployed to Preview July 1, 2020 04:25 Inactive
@liamtoney
Copy link
Member Author

Two minor comments on this:

  1. I didn't actually preview the ReST of the docstring. Do we have the math extension? Otherwise the :math: directive won't render properly.

  2. I tried to illuminate the surface, but I='+' gave a flat image with shading rather than an illuminated 3-D surface.

@weiji14
Copy link
Member

weiji14 commented Jul 1, 2020

  1. I didn't actually preview the ReST of the docstring. Do we have the math extension? Otherwise the :math: directive won't render properly.

It might be tough for Scipy Sprint folks to preview the documentation locally, unless they've set up a PyGMT environment, instructions are at https://github.com/GenericMappingTools/pygmt/blob/master/CONTRIBUTING.md#documentation if anyone needs them.

Luckily we've got #344, which reminds me, I'll send you an email to invite you as an owner on the Vercel gmt team, since you're part of @GenericMappingTools/python now. Anyways, preview is at https://pygmt-git-fork-liamtoney-grdview-gallery.gmt.now.sh/gallery/grid/grdview_surface.html#plotting-a-surface, or you can click on the "View deployment" button

Vercel View deployment button

  1. I tried to illuminate the surface, but I='+' gave a flat image with shading rather than an illuminated 3-D surface.

Haven't tried illumination with grdview before, maybe try with debug mode on using V="d"?

@vercel vercel bot temporarily deployed to Preview July 1, 2020 04:42 Inactive
@liamtoney
Copy link
Member Author

Anyways, you can preview things at https://pygmt-git-fork-liamtoney-grdview-gallery.gmt.now.sh/gallery/grid/grdview_surface.html#plotting-a-surface.

That is really nifty!

Haven't tried illumination with grdview before, maybe try with debug mode on using V="d"?

Didn't see anything obvious in the console output. Will have to table for later, it looks alright as is!

@liamtoney
Copy link
Member Author

liamtoney commented Jul 1, 2020

Didn't see anything obvious in the console output. Will have to table for later, it looks alright as is!

I was able to re-create the code in pure GMT, and the shading performs as expected, so it's a PyGMT grid I/O issue I think. I'll make an issue.

Edit: I think this issue is the same as #364, so I'll just comment there.

@seisman seisman added the documentation Improvements or additions to documentation label Jul 1, 2020
Co-authored-by: Dongdong Tian <[email protected]>
@vercel vercel bot temporarily deployed to Preview July 2, 2020 04:33 Inactive
Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@seisman seisman added this to the 0.1.x milestone Jul 2, 2020
@seisman
Copy link
Member

seisman commented Jul 2, 2020

@weiji14 is the one implementing the grdview() functions. Any comments?

@liamtoney liamtoney merged commit 294aee8 into GenericMappingTools:master Jul 2, 2020
@liamtoney liamtoney deleted the grdview-gallery branch July 2, 2020 20:24
@liamtoney
Copy link
Member Author

Whoops. I saw your approval and merged. Apologies if that was premature.

@seisman
Copy link
Member

seisman commented Jul 2, 2020

It's fine. We can fix it later if @weiji14 wants more.

@weiji14
Copy link
Member

weiji14 commented Jul 2, 2020

Oops, missed it by a few minutes. I actually really like this Ackley function example, definitely better than anything I could have come up with!

Aside from a few extra inline comments, I think it's fine as is. We can add them in when the shading works in GMT 6.1 😉

@liamtoney
Copy link
Member Author

We can add them in when the shading works in GMT 6.1 😉

Yes the shaded version looks quite nice so I'd like to update this example once we get to 6.1!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation scipy-sprint Things to work on during the SciPy sprint!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants