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

WIP Refactor test for grdsample #4667

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

WIP Refactor test for grdsample #4667

wants to merge 5 commits into from

Conversation

PaulWessel
Copy link
Member

Description of proposed changes

This PR breaks up grdsample.c's GMT_grdsample into a new function gmtlib_grd_sample that does the work, while GMT_grdsample deals with the i/o, arguments, options and then calls gmtlib_grd_sample. All tests still pass so done OK I think. The prototype looks like this:

int gmtlib_grd_sample (void *API, struct GMT_GRID *Gin, double *wesn, double *incs, int registration, int mode, struct GMT_GRID **G);

Yet there are questions:

  • We pass API as first argument, like for all API functions. Alternative is a laundry-list of settings and option arguments.
  • We pass in a GMT_GRID structure and receives a new one back. If externals have not wrapped such structures but use matrices, xarrays, etc, then what should the interface be? The current one is the closest to the GMT API without doing any i/o. Do we add additional C wrappers called gmtlib_matrix_sample that now need to pass many more arguments (basically the info in the grid headers, in and out, plus a 2-D array? Does that simplify anything? Seems it gets more complicated if each wrapper has to use a container specific to its language.
  • We still have the pad issue. Resampling requires the 2 boundary pads which all GMT internal grids have. But externals will pass in grids without pads and then we have to duplicate memory to add pads.

Extract a separate gmtlib_grd_sample function in grdsample.c that does most of the heavy lifting and is called by GMT_grdsample.
@PaulWessel PaulWessel added the discussion Topics for longer discussion label Jan 18, 2021
@PaulWessel PaulWessel requested a review from a team January 18, 2021 19:50
@PaulWessel PaulWessel self-assigned this Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Topics for longer discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant