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 Move this supplement to 6.1 support #2

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

Conversation

PaulWessel
Copy link
Member

Rely on main GMT's configuration machinery. Explore if this is a type 4 supplement or type 2.

Rely on main GMT's configuration machinery.
@PaulWessel
Copy link
Member Author

I think the custom-supplements should be a straight type 4 supplement: Only have dependency on the official GMT API (gmt.h). If that is the goal (we have other examples of type 1-3) then what this supplement needs (I think) is:

  1. A simple CMakeLists.txt file that uses find_package (GMT REQUIRED) to find the GMT library and include files as they are needed.
  2. Just build the custom.so library.

Without gmt_dev.h there is no glue functions since the gmtlib_module_show_all and other functions are not in the API but in the developer library. So custom.so cannot display anything via gmt --help and similar, but you can still run gmt customtool and we will look for customtool in any of the shared libraries GMT knows about (i.e., in the official plugin dir or via GMT_CUSTOM_LIBS).

The alternative solution to this is to elevate the glue functions to the API. The way I would do that is to introduce two new API functions

void GMT_Module_Display (void *API, struct GMT_MODULEINFO *M, char *arg, unsigned int mode);
const char *GMT_Module_Info (void *API, char *module, unsigned int mode);

and 5 new enums:

enum GMT_enum_suppl {
	GMT_SUPPL_HELP    = 0,	/* Display all modules in gmt --help */
	GMT_SUPPL_SHOW_MODERN    = 1,	/* Display all modern modules in gmt --show-modules */
	GMT_SUPPL_SHOW_CLASSIC   = 2,	/* Display all classic modules in gmt --show-classic */
	GMT_SUPPL_KEYS   = 0,	/*Request the module keys */
	GMT_SUPPL_GROUP   = 1	/*Request the module group */
};

This means the glue would only need gmt.h and have code like this instead:

/* Pretty print all shared module names and their purposes for gmt --help */
EXTERN_MSC void mbsystem_module_show_all (void *API) {
	GMT_Module_Display (API, modules, "GMT core: The main modules of the Generic Mapping Tools", GMT_SUPPL_HELP);
}

etc. I am not sure if this is worth the work since as far as I know there are no type 4 supplements in the wild. Having those use gmt_dev.h instead does not really impose much hardships since all GMT distros install all the developer includes. It would be up to us to insist on separating gmt and gmt-dev in the ubuntos, etc. Thoughts, @seisman and @joa-quim ?

@seisman
Copy link
Member

seisman commented Apr 26, 2020

We cannot access type 4 modules in Matlab, Python or Julia, right? I think this is the biggest limitation of type 4.

@joa-quim
Copy link
Member

Type 4 suppls should be accessible from Matlab/Julia (don't know how the Python wrapper works) but only API functions will be at reach. Don't really see the interest of this limitation. Anyone building supplements should be at ease to use gmt_dev.h, even if not always needed.

@PaulWessel
Copy link
Member Author

Hm, maybe yes and no? My ignorance may be showing because I have not looked at the pyGMT code, but I think each module is wrapped individually, so you can do some things differently for each module? in GMTMEX we chose the abstract model where the gmtmex.c code does not know anything about what modules do and how they are called, and instead rely on the user giving a sensible command string and the KEYS to complete that string. So for GMTMEX: Yes, could not work with type 4 supplements, but if the pyGMT developer knows what a type 4 module does they could write a wrapper since in the end there is just a GMT_Call_Module that runs the module.

@joa-quim
Copy link
Member

Does a type 4 clock the access to the KEYs? (Then, I agree T4 is a waste and would promote crippled supplements)

@PaulWessel
Copy link
Member Author

Type 4 cannot access KEYS; see my proposal earlier to add API functions to get to keys.

@joa-quim
Copy link
Member

Then, either that or no T4 at all.

@PaulWessel
Copy link
Member Author

I guess adding the two new API function is preferable since it will allow all types 1-4 to be accessed fully (i.e., with keys and usage) in gmt. I will add those functions.

seisman added 4 commits April 30, 2020 23:35
* Update the root CMakeLists.txt

* Add CMake codes for building the custom library

* Copy FindGMT and FindNETCDF from the GMT repository

* Add gmt_gule.c, generated by gmt --new-glue

* GMT_Get_Value was renamed to GMT_Get_Values and has one more parameter

* custom_version.h was removed

* GMT_STR16 was renamed to GMT_VF_LEN

* GMT_GRD_HEADER nx and ny was renamed to n_columns and n_rows

* Fix GMT_Get_Values in gmtparser.c
@seisman
Copy link
Member

seisman commented May 1, 2020

Perhaps we should have type 4 and type 5 supplements.

  • Type 4 supplements include "gmt_dev.h" and can access all the exported APIs (GMT_* and gmt_*). It's different from type 2 and can be built after GMT is installed.
  • Type 5 supplements include "gmt.h" and can only access the public APIs (i.e., GMT_*). I can see some developers may prefer type 5 to type 4, because only the public APIs are guaranteed to be stable. Another difference between types 4 and 5 is that type 4 needs to find GMT, netCDF and GDAL libraries, whereas type 5 only needs the GMT library.

Currently, the supplement is mixed with types. gmtaverage and gmtmercmap are type 4. gmtparser and grdfourier belong to type 5.

Here are some remaining issues:

  1. For type 5, gmt_show_name_and_purpose is not declared.
  2. Debug messages say
mt [DEBUG]: Shared Library # 1 (supplements). Path = /Users/seisman/local/GMT/lib/gmt/plugins/supplements.so
gmt [DEBUG]: Shared Library # 2 (custom). Path =

the path to the custom library is empty.

@PaulWessel
Copy link
Member Author

Yes, that makes sense. Perhaps we should split custom_supplements into two different repos since they have different purposes and prereqs.
Sorry, need to bury my head in issue 3196 in GMT (how do we cross-issue cite?)

@seisman
Copy link
Member

seisman commented May 1, 2020

Sorry, need to bury my head in issue 3196 in GMT (how do we cross-issue cite?)

Just copy and paste the full URL of that issue. I think you're talking about
GenericMappingTools/gmt#3130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants