Skip to content

Wrap sphdistance - #1383

Merged
seisman merged 16 commits into
mainfrom
wrap-sphdistance
Sep 10, 2021
Merged

Wrap sphdistance#1383
seisman merged 16 commits into
mainfrom
wrap-sphdistance

Conversation

@willschlitzer

@willschlitzerwillschlitzer commented Jul 11, 2021

Copy link
Copy Markdown
Contributor

This pull request wraps the gmt module sphdistance.

Fixes #

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

@willschlitzerwillschlitzer added the feature Brand new feature label Jul 11, 2021
@willschlitzerwillschlitzer added this to the 0.5.0 milestone Jul 11, 2021
@willschlitzerwillschlitzer self-assigned this Jul 11, 2021
@willschlitzerwillschlitzer mentioned this pull request Jul 29, 2021
Merged
5 tasks
@willschlitzerwillschlitzer changed the title WIP: Wrap sphdistanceWrap sphdistanceAug 8, 2021
@willschlitzer
willschlitzer marked this pull request as ready for review August 11, 2021 08:54
weiji14
weiji14 reviewed Sep 2, 2021
View reviewed changes
Comment threadpygmt/src/sphdistance.py Outdated
Comment threadpygmt/src/sphdistance.py Outdated
Comment threadpygmt/src/sphdistance.py
Comment threadpygmt/src/sphdistance.py Outdated
Comment threadpygmt/tests/test_sphdistance.py Outdated
@willschlitzer@weiji14
Apply suggestions from code review
d75ada0
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
weiji14
weiji14 approved these changes Sep 9, 2021
View reviewed changes

@weiji14weiji14 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks alright to me 😁

@weiji14weiji14 added the final review call This PR requires final review and approval from a second reviewer label Sep 9, 2021
seisman
seisman reviewed Sep 9, 2021
View reviewed changes
Comment threadpygmt/src/sphdistance.py Outdated
@willschlitzer@seisman
Apply suggestions from code review
b247dc4
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
seisman
seisman reviewed Sep 9, 2021
View reviewed changes
Comment threadpygmt/src/sphdistance.py Outdated
Comment threadpygmt/tests/test_sphdistance.py Outdated
Comment threadpygmt/tests/test_sphdistance.py Outdated
@willschlitzer@seisman
Apply suggestions from code review
5f35b08
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
seisman
seisman approved these changes Sep 9, 2021
View reviewed changes

@seismanseisman left a comment

Copy link
Copy Markdown
Member

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 merged commit 2edaa63 into mainSep 10, 2021
@seisman
seisman deleted the wrap-sphdistance branch September 10, 2021 05:08
@seismanseisman removed the final review call This PR requires final review and approval from a second reviewer label Sep 10, 2021
@maxrjones

maxrjones commented Sep 10, 2021

Copy link
Copy Markdown
Member

Sorry for the comment after merging, but I think this needs to be in 'operations on tabular data' rather than 'operations on grids'.

Also the 'table' parameter is missing from the docstring (#1479 (comment) is relevant here).

@willschlitzer

willschlitzer commented Sep 10, 2021

Copy link
Copy Markdown
ContributorAuthor

Sorry for the comment after merging, but I think this needs to be in 'operations on tabular data' rather than 'operations on grids'.

Also the 'table' parameter is missing from the docstring (#1479 (comment) is relevant here).

I'll open a PR to fix it

This was referenced Sep 11, 2021
Merged
Merged
@weiji14weiji14 mentioned this pull request Sep 18, 2021
Merged
31 tasks
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
@willschlitzer@weiji14
@seismanJosh Sixsmith
Wrap sphdistance (GenericMappingTools#1383)
1c9eb3a
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
@seismanseisman mentioned this pull request Apr 11, 2026
Closed
4 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seismanseismanseisman approved these changes
@weiji14weiji14weiji14 approved these changes

Assignees

@willschlitzerwillschlitzer

Labels

featureBrand new feature

Projects

None yet

Milestone

0.5.0

Development

Successfully merging this pull request may close these issues.

4 participants

@willschlitzer@maxrjones@seisman@weiji14