-
Notifications
You must be signed in to change notification settings - Fork 224
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 Mars dataset #1420
Add Mars dataset #1420
Conversation
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.
Just one minor docstring suggested change to apply (you'll need to resolve the merge conflict with load_hotspot
first though).
Catching up on the discussion at #1436 (comment), if we're moving to follow the seaborn.load_dataset
model, then this PR would need substantial changes. Do you want to do the refactoring here or separately?
Co-authored-by: Wei Ji <[email protected]>
My thought is to handle it with a separate PR. While I don't want to duplicate efforts, this seems like a pretty substantial change for follow |
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.
Just one minor docstring suggested change to apply (you'll need to resolve the merge conflict with
load_hotspot
first though).
Catching up on the discussion at #1436 (comment), if we're moving to follow theseaborn.load_dataset
model, then this PR would need substantial changes. Do you want to do the refactoring here or separately?My thought is to handle it with a separate PR. While I don't want to duplicate efforts, this seems like a pretty substantial change for follow
seaborn.load_dataset
, and I think it's easier for this to be merged first before it is refactored to follow a new way of doing things.
Ok to approve then, and refactor later.
/format |
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.
Looks good to me.
@willschlitzer, before you merge this, could you please add an entry under https://github.com/GenericMappingTools/pygmt/blob/main/doc/api/index.rst#datasets? (Also would be nice to sort the entries alphabetically). Thanks! |
Sample data table for the shape of Mars. This is the `@mars370d.txt` dataset used in GMT examples, with data and information from Smith, D. E., and M. T. Zuber (1996). The shape of Mars and the topographic signature of the hemispheric dichotomy. Data columns are "longitude", "latitude", and "radius (meters)." Co-authored-by: Wei Ji <[email protected]>
This pull request adds a function to the dataset samples for the
mars370d.txt
file used in thesphinterpolate
example.Fixes #
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version