Skip to content

Session.get_libgmt_func: Let restype default to C int type and use None for void - #4859

Open
seisman wants to merge 1 commit into
mainfrom
clib/get_libgmt_func
Open

Session.get_libgmt_func: Let restype default to C int type and use None for void#4859
seisman wants to merge 1 commit into
mainfrom
clib/get_libgmt_func

Conversation

@seisman

@seisman seisman commented Aug 25, 2026

Copy link
Copy Markdown
Member

This PR simplifies the Session.get_libgmt_func and fixes a minor issue:

  1. self._libgmt is assigned but not used. It was useful in old versions, but not needed after we load the gGMT library globally after Improve performance by avoiding loading the GMT library repeatedly #2930.
  2. By default, ctypes assumes function return the C int type (xref: https://docs.python.org/3/library/ctypes.html#return-types), and setting restype=None is equivalent to setting restype=ctp.c_void_p (xref: https://docs.python.org/3/library/ctypes.html#return-types). However, in Session.get_libgmt_func, restype=None means not to change restype, making the behavior inconsistent with the ctypes's default behavior. This PR fixes it. It doesn't break anything, mainly because we use restype=ctp.c_void_p when calling this method.

Detected and fixed by Claude; Reviewed by @seisman.

- Read the shared library directly from the module-level '_libgmt' instead of
  lazily copying it onto 'self._libgmt', which nothing ever reassigned.
- Default 'restype' to 'ctypes.c_int' (already ctypes' default) so that passing
  'restype=None' means a void return, as it does in ctypes. All 17 call sites
  pass 'restype' explicitly, so behavior is unchanged.
- Fix the 'restype' docstring: "the input returned" -> "the value returned", and
  move its type into the signature to match the other parameters.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@seisman seisman added this to the 0.20.0 milestone Aug 25, 2026
@seisman seisman added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog needs review This PR has higher priority and needs review. bug Something isn't working and removed maintenance Boring but important stuff for the core devs bug Something isn't working labels Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Boring but important stuff for the core devs needs review This PR has higher priority and needs review. skip-changelog Skip adding Pull Request to changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant