Wrap project

[claudiodsf]

Description of proposed changes

⚠️ This is my first try at wrapping a GMT function. ⚠️

Here I propose a wrapper for project.

Still some stuff to do (tests!) and many questions, which I'll ask in a comment below.

Preview docs at https://pygmt-git-fork-claudiodsf-project-gmt.vercel.app/api/generated/pygmt.project.html

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

[claudiodsf]

Let's go with the questions!

1. I'm confused on whether I should use a pandas dataframe or a NumPy array as input. I saw both in the existing code and I could not understand the rationale between using one or the other (so, for the moment, I'm using pandas).
2. I'm in a special case where I have a required keyword parameter (center): this prevents me to use the @kwargs_to_strings decorator and I had to do a little hack. Any better idea?
3. I put points as first parameter for consistency with other wrapped functions. However this parameter is ignored if generate is used. Not sure if the solution I came with is nice...

[seisman]

1. I'm confused on whether I should use a pandas dataframe or a NumPy array as input. I saw both in the existing code and I could not understand the rationale between using one or the other (so, for the moment, I'm using pandas).

Good question. I think we should always use NumPy if possible, but some modules (e.g., blockmean) may only support pandas dataframe. I think we should fix it.

2. I'm in a special case where I have a required keyword parameter (center): this prevents me to use the @kwargs_to_strings decorator and I had to do a little hack. Any better idea?

If you add C="center" to @use_alias and C="sequence" to @kwargs_to_strings, it might work.

3. I put points as first parameter for consistency with other wrapped functions. However this parameter is ignored if generate is used. Not sure if the solution I came with is nice...

Perhaps you can set points=None and check if points is given when generate is not used?

[claudiodsf]

Thanks @seisman for your comments.

Let me reply to each of them:

Good question. I think we should always use NumPy if possible, but some modules (e.g., blockmean) may only support pandas dataframe. I think we should fix it.

What about accepting both NumPy arrays and pandas dataframes? The function return value could be adjusted accordingly. For the specific case of project, dataframes are quite handy to keep track of column names.

If you add C="center" to @use_alias and C="sequence" to @kwargs_to_strings, it might work.

It doesn't work because center is a required argument and has to be positional, not keyword.
My understanding is that the decorator @kwargs_to_strings only deals with keyword arguments.

Maybe a solution would be using required keyword-only arguments:

def project(points, *, center, outfile=None, **kwargs)

but it doesn't seem to be supported by @kwargs_to_strings.

Perhaps you can set points=None and check if points is given when generate is not used?

This will conflict with the second argument (center) which is required. A solution would be moving points to second argument, but this would be inconsistent with other functions. Let me know what you prefer.

[seisman]

Good question. I think we should always use NumPy if possible, but some modules (e.g., blockmean) may only support pandas dataframe. I think we should fix it.

What about accepting both NumPy arrays and pandas dataframes? The function return value could be adjusted accordingly. For the specific case of project, dataframes are quite handy to keep track of column names.

I meant the input should be general enough to support string (filenames), 2D list, 2D numpy array and pandas.DataFrame. For output, pandas.DataFrame sounds good. For people who want numpy, perhaps they should just convert the pandas.DataFrame to numpy using pandas.DataFrame.to_numpy()?

[willschlitzer]

Ping @claudiodsf to check in and see if you need any help getting this pull request to completion

[claudiodsf]

Ping @claudiodsf to check in and see if you need any help getting this pull request to completion

Hi @willschlitzer and thanks for your help.

There are two pending questions for this PR:

1. Should points be a NumPy array, a Pandas data frame, or should we support every kind of input. This seems to vary from function to function in PyGMT, but maybe you have now defined a strategy?
2. Should we keep points as first argument (for consistency with other PyGMT functions) or move it after center, given that center is a required argument and points is not?

Please note that I'm leaving on summer break next Wednesday and will have little time to work on this PR till then. I'll be back on the second half of August.

[willschlitzer]

1. Should points be a NumPy array, a Pandas data frame, or should we support every kind of input. This seems to vary from function to function in PyGMT, but maybe you have now defined a strategy?

I would give it the option to accept multiple types of table-like inputs, such as a list, numpy array, or DataFrame. Have a look at grdtrack on how to accept multiple types of table-like inputs.

2. Should we keep points as first argument (for consistency with other PyGMT functions) or move it after center, given that center is a required argument and points is not?

Since neither points or center are in kwargs, I don't think the order matters too much. That being said, to update the discussion from your comments earlier about having C="center" in the kwargs, I would still recommend including it in one of the aliases, and then have some sort of check to ensure that C is in kwargs; having required arguments in the kwargs is common for PyGMT modules. Something like:

if "C" not in kwargs.keys():
    raise GMTInvalidInput("'center' must be specified.")

Please note that I'm leaving on summer break next Wednesday and will have little time to work on this PR till then. I'll be back on the second half of August.

No worries! Just wanted to check in to make sure this PR isn't forgotten about. Enjoy your summer break; PyGMT will still need work to be done when you get back!

[weiji14]

Hi @claudiodsf, we're planning to release PyGMT v0.5.0 in about two weeks (~15 October 2021). Let us know if you have time to finish this Pull Request, otherwise we can get one of the maintainers to finalize things and get this feature in 😄

[maxrjones]

Hi @claudiodsf, we're planning to release PyGMT v0.5.0 in about two weeks (~15 October 2021). Let us know if you have time to finish this Pull Request, otherwise we can get one of the maintainers to finalize things and get this feature in 😄

I am available to help with finalizing this pull request. @claudiodsf, would you mind if I push changes to this branch in order to include your new feature in v0.5.0?

[claudiodsf]

Hi @meghanrjones,

sure! Feel free to push your changes.

Sorry for slow replying, but it's a tough working period.

[maxrjones]

Hi @meghanrjones,

sure! Feel free to push your changes.

Sorry for slow replying, but it's a tough working period.

Great, thanks. And no problem, completely understand!

[willschlitzer]

@GenericMappingTools/pygmt-maintainers Could someone take a look at the suggestions I made for the project documentation and give it a yes/no? I want to get this pull request done for v0.5.0, and as @meghanrjones is currently on vacation (:tada:), I don't want to be the one approving my own suggestions.

[maxrjones]

/format

[willschlitzer]

Approved with one change recommendation

[maxrjones]

Thanks again for this feature @claudiodsf! Excited to have this out in the v0.5.0 release 🎉! As a reminder, you're welcome to submit a PR adding yourself to AUTHORS.md.