Update grdfill parameters for GMT 6.2.0

[willschlitzer]

This pull request adds additional parameters to the grdfill module.

Continuation of work on #1276

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

[weiji14]

Just surfacing a point made in #1276:

What is the preferred was for denoting required parameters? I think there should be some indication in the method docs that mode (-A) is required.

Perhaps just a one-line description in the docstring wll suffice?

Could you please add a line in the docstring that says mode is a required parameter? I'll also leave it up to you to decide whether a GMTInvalidInput error should be raised if mode is not provided (requires coding up a few lines).

Edit: Maybe hold on first, -A might not be a required parameter in grdfill, see GenericMappingTools/gmt#5247 (comment).

[willschlitzer]

Just surfacing a point made in #1276:

What is the preferred was for denoting required parameters? I think there should be some indication in the method docs that mode (-A) is required.

Perhaps just a one-line description in the docstring wll suffice?

Could you please add a line in the docstring that says mode is a required parameter? I'll also leave it up to you to decide whether a GMTInvalidInput error should be raised if mode is not provided (requires coding up a few lines).

Edit: Maybe hold on first, -A might not be a required parameter in grdfill, see GenericMappingTools/gmt#5247 (comment).

It looks like @PaulWessel is planning on making -A required. I'm not sure what the timeline is for that, so I added a test to check that grdfill works with only a grid argument.

[maxrjones]

Another update from that gmt grdfill PR: either -A or -L is required. The bug fix will be merged after 6.2.0rc2 and likely before 6.2.0.

I think the best solution for pygmt is to check that either -A or -L are provided and give a GMTInvalidInput error if not (similar to pygmt/src/coast.py L190-194 and the corresponding test in pygmt/tests/test_coast.py L36 - 42).

[willschlitzer]

Another update from that gmt grdfill PR: either -A or -L is required. The bug fix will be merged after 6.2.0rc2 and likely before 6.2.0.

I think the best solution for pygmt is to check that either -A or -L are provided and give a GMTInvalidInput error if not (similar to pygmt/src/coast.py L190-194 and the corresponding test in pygmt/tests/test_coast.py L36 - 42).

I made A and L required arguments, but the error that is raised only specifies A being required, as I haven't wrapped L in this pull request. I couldn't figure out how to get around it not returning a grid object/file.

[core-man]

Looks great~

[seisman]

@meghanrjones Could you please give this PR a review, since you know more about the changes on the GMT side.

[maxrjones]

Is the plan to add -L separately?

The s option for mode should eventually be added (it was implemented before 6.2.0rc2 - just was a documentation error that stated it was not yet an option). This could be a separate PR if needed.

[seisman]

Is the plan to add -L separately?

Option -L output some information to stdout, rather than modifying an input grid. I believe it needs more effort to think about how to return the -L output and can be done in a separate PR.

The s option for mode should eventually be added (it was implemented before 6.2.0rc2 - just was a documentation error that stated it was not yet an option). This could be a separate PR if needed.

I think this documentation issue is small and should be included into this PR.

[seisman]

Ping @willschlitzer to improve the description of -A option, that mode="s" is allowed (See https://docs.generic-mapping-tools.org/dev/grdfill.html).

[willschlitzer]

Ping @willschlitzer to improve the description of -A option, that mode="s" is allowed (See https://docs.generic-mapping-tools.org/dev/grdfill.html).

I'm planning on doing this after I get back to my home on June 14th.

[weiji14]

Welcome back @willschlitzer! Hope you had some good time off. Just one review comment for now 😃

[maxrjones]

One tiny comment, otherwise looks great!