Skip to content

Conversation

@daroari
Copy link
Contributor

@daroari daroari commented Aug 19, 2021

Description of proposed changes

This PR expands the table-like input options for pygmt.surface

Closes #1443

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

Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
@maxrjones
Copy link
Member

/format

@weiji14 weiji14 added the enhancement Improving an existing feature label Aug 20, 2021
Copy link
Member

@maxrjones maxrjones left a comment

Choose a reason for hiding this comment

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

Great work!

Can you please remove this line from /pygmt/src/surface.py in your branch? This function is no longer used in the module, so it does not need to be imported:

dummy_context,

@seisman
Copy link
Member

seisman commented Aug 29, 2021

Ping @daroari to address the comment #1455 (review)

@seisman seisman added this to the 0.5.0 milestone Aug 29, 2021
@seisman
Copy link
Member

seisman commented Sep 3, 2021

/format

@maxrjones
Copy link
Member

Hi @daroari, do you think you will have time to finish this pull request? It would be great to get it added to PyGMT 😄

If not, we can either close the PR or I could push the remaining changes needed to this branch, whichever you prefer.

@daroari
Copy link
Contributor Author

daroari commented Sep 21, 2021

Hi @meghanrjones. Sorry for the delayed response. Would you please push the remaining changes? I was going to do it, but I am not able to do it right now. Sorry for the inconvenience :(

@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label Sep 21, 2021
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Thumbs up from me. Technically, it would be nice to have a unit test to make sure this works (e.g. using pytest.mark.parametrize like for blockm* in #1523), but ok to do it another time too 🙂

Copy link
Member

@maxrjones maxrjones left a comment

Choose a reason for hiding this comment

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

Thanks @daroari for starting the work and @seisman for finishing it up!

@maxrjones maxrjones merged commit 3840803 into GenericMappingTools:main Sep 21, 2021
@maxrjones maxrjones added eswn-workshop Good issues for first-contributions during the ESWN-PyGMT workshop and removed final review call This PR requires final review and approval from a second reviewer labels Sep 27, 2021
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…s#1455)

* Expand table-like input options for pygmt.surface

* Remove unused imports and variables

* Update docstring for data

Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improving an existing feature eswn-workshop Good issues for first-contributions during the ESWN-PyGMT workshop

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand table-like input options for pygmt.surface

5 participants