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

Support 1d array for outcols in grdtrack and blockm functions #1524

Merged
merged 2 commits into from
Sep 19, 2021

Conversation

maxrjones
Copy link
Member

@maxrjones maxrjones commented Sep 19, 2021

Description of proposed changes

This PR adds support for 1d array for outcols in grdtrack and blockm* functions by updating kwargs_to_strings arguments.

Addresses #1445.

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

Sorry, something went wrong.

@seisman seisman added this to the 0.5.0 milestone Sep 19, 2021
@seisman seisman added the enhancement Improving an existing feature label Sep 19, 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.

Looks ok to me, but maybe rename the title to something like "Support list inputs for outcols parameter in grdtrack and blockm functions". The "1d array" sounds like people will do outcols=np.array([0,1]), but the sequence comma is really just for doing outcols=[0,1].

@seisman
Copy link
Member

seisman commented Sep 19, 2021

The "1d array" sounds like people will do outcols=np.array([0,1]), but the sequence comma is really just for doing outcols=[0,1].

I think outcols=np.array([0, 1]) is also supported.

@weiji14
Copy link
Member

weiji14 commented Sep 19, 2021

The "1d array" sounds like people will do outcols=np.array([0,1]), but the sequence comma is really just for doing outcols=[0,1].

I think outcols=np.array([0, 1]) is also supported.

Yes it'll work, but a bit overkill...

Also, does this PR need the skip-changelog label? I don't think outcols was added in PyGMT v0.4.1.

@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label Sep 19, 2021
@seisman seisman added the skip-changelog Skip adding Pull Request to changelog label Sep 19, 2021
@maxrjones
Copy link
Member Author

Looks ok to me, but maybe rename the title to something like "Support list inputs for outcols parameter in grdtrack and blockm functions". The "1d array" sounds like people will do outcols=np.array([0,1]), but the sequence comma is really just for doing outcols=[0,1].

I think that I'll keep it as written because docstrings describe the accepted argument types as (str or 1d array).

@maxrjones maxrjones merged commit 2d2eb65 into main Sep 19, 2021
@maxrjones maxrjones deleted the outcols-sequence branch September 19, 2021 19:27
@maxrjones maxrjones removed the final review call This PR requires final review and approval from a second reviewer label Sep 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving an existing feature skip-changelog Skip adding Pull Request to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants