-
Notifications
You must be signed in to change notification settings - Fork 245
Wrap GMT_Put_Strings to pass str columns into GMT C API directly #520
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
Changes from 4 commits
c811e65
5c5f152
d7c3053
fccedae
07b95ed
88eef3a
6390e04
42af00e
9ef04b0
ab1e041
01754fc
452d3d3
9c8dad9
6dff9ba
cfb9124
2485437
7fa1f0c
e8e7768
2f0c798
6d6ea24
9d063b3
2d81f76
95afa90
ed0dce6
aeabd8e
abcbcf7
129135e
4f1ccda
30c18eb
62469cb
26f51c9
726de44
4c99477
2f9d689
fe794c9
f59d970
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,7 @@ | |
| np.uint64: "GMT_ULONG", | ||
| np.uint32: "GMT_UINT", | ||
| np.datetime64: "GMT_DATETIME", | ||
| np.str_: "GMT_TEXT", | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -235,7 +236,7 @@ def __getitem__(self, name): | |
| value = c_get_enum(session, name.encode()) | ||
|
|
||
| if value is None or value == -99999: | ||
| raise GMTCLibError("Constant '{}' doesn't exits in libgmt.".format(name)) | ||
| raise GMTCLibError(f"Constant '{name}' doesn't exist in libgmt.") | ||
|
|
||
| return value | ||
|
|
||
|
|
@@ -731,7 +732,7 @@ def put_vector(self, dataset, column, vector): | |
| """ | ||
| Attach a numpy 1D array as a column on a GMT dataset. | ||
|
|
||
| Use this functions to attach numpy array data to a GMT dataset and pass | ||
| Use this function to attach numpy array data to a GMT dataset and pass | ||
| it to GMT modules. Wraps ``GMT_Put_Vector``. | ||
|
|
||
| The dataset must be created by :meth:`~gmt.clib.Session.create_data` | ||
|
|
@@ -791,11 +792,62 @@ def put_vector(self, dataset, column, vector): | |
| ) | ||
| ) | ||
|
|
||
| def put_strings(self, dataset, column, strings): | ||
| """ | ||
| Attach a numpy 1D array of dtype str as a column on a GMT dataset. | ||
|
|
||
| Use this function to attach string type numpy array data to a GMT | ||
| dataset and pass it to GMT modules. Wraps ``GMT_Put_Strings``. | ||
|
|
||
| The dataset must be created by :meth:`~gmt.clib.Session.create_data` | ||
| first. Use ``family='GMT_IS_DATASET|GMT_VIA_VECTOR'``. | ||
|
|
||
| .. warning:: | ||
| The numpy array must be C contiguous in memory. If it comes from a | ||
| column slice of a 2d array, for example, you will have to make a | ||
| copy. Use :func:`numpy.ascontiguousarray` to make sure your vector | ||
| is contiguous (it won't copy if it already is). | ||
|
|
||
| Parameters | ||
| ---------- | ||
| dataset : :class:`ctypes.c_void_p` | ||
| The ctypes void pointer to a ``GMT_Dataset``. Create it with | ||
| :meth:`~gmt.clib.Session.create_data`. | ||
| column : int | ||
| The column number of this vector in the dataset (starting from 0). | ||
|
weiji14 marked this conversation as resolved.
Outdated
|
||
| strings : numpy 1d-array | ||
| The array that will be attached to the dataset. Must be a 1d C | ||
| contiguous array. | ||
|
|
||
| Raises | ||
| ------ | ||
| GMTCLibError | ||
| If given invalid input or ``GMT_Put_Strings`` exits with status != | ||
| 0. | ||
|
|
||
| """ | ||
| c_put_strings = self.get_libgmt_func( | ||
| "GMT_Put_Strings", | ||
| argtypes=[ctp.c_void_p, ctp.c_uint, ctp.c_void_p, ctp.c_uint], | ||
|
seisman marked this conversation as resolved.
Outdated
|
||
| restype=ctp.c_int, | ||
| ) | ||
|
|
||
| gmt_type = self._check_dtype_and_dim(strings, ndim=1) | ||
| strings_pointer = strings.ctypes.data_as(ctp.c_void_p) | ||
| status = c_put_strings( | ||
| self.session_pointer, dataset, column, gmt_type, strings_pointer | ||
| ) | ||
|
Comment on lines
+829
to
+853
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes I tried this following your datetime example, but I think (?) it's functionally equivalent to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What I did is adding a print statement inside the GMT_Put_Strings function, printing the string arrays passed to it. Using
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about using: c_put_strings = self.get_libgmt_func(
"GMT_Put_Strings",
argtypes=[
ctp.c_void_p,
ctp.c_uint,
ctp.c_void_p,
ctp.POINTER(ctp.c_char_p),
],
restype=ctp.c_int,
)
strings_pointer = (ctp.c_char_p * len(strings))()
strings_pointer[:] = np.char.encode(strings)Does it crash for you?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it still crashes, but I can see that the strings are correctly passed to the GMT_Put_Strings function.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I was just taking a look at it (thanks for tracking down the bug by the way!). Sounds like we'll need to pin to GMT > 6.1.1 for the next release?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually I'm not 100% sure GenericMappingTools/gmt#3718 is related to the issue here. I just had the feeling that the string array may be freed by Python when GMT tries to read it. Hopefully, GMT 6.1.1 can fix issue and #515.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI, GenericMappingTools/gmt#3718 is already merged into GMT master, and will be backported to 6.1 branch soon.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great! I'll test it out when I get the time (need to prepare some stuff for an online conference next week). Still need to wait on the grid problem at #515 but that's a separate issue.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just triggered the GMT master branch testing. Ideally, we should only see one failure from
Comment on lines
+842
to
+853
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the upstream PR GenericMappingTools/gmt#3718 and some changes in the Python code (I don't push the changes yet), the For GMT_Put_Vector, GMT_Put_Matrix, GMT_Put_Strings, they don't duplicate the vector, matrix or strings by default. So if the vector, matrix, or strings are freed in Python, we should see some unexpected behaviors or crashes. The situation never happens for GMT_Put_Vector and GMT_Put_Matrix, but happens for GMT_Put_Strings. vectors/matrix and strings are passed using different ways. Here is how we pass vectors, here is how we pass strings) As I understand it, For strings, if we have a way to pass data just like vectors or matrix, then we probably don't need PR #3718, which duplicates the strings.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally we could just do |
||
| if status != 0: | ||
| raise GMTCLibError( | ||
| f"Failed to put strings of type {strings.dtype}", | ||
| f"in column {column} of dataset.", | ||
| ) | ||
|
|
||
| def put_matrix(self, dataset, matrix, pad=0): | ||
| """ | ||
| Attach a numpy 2D array to a GMT dataset. | ||
|
|
||
| Use this functions to attach numpy array data to a GMT dataset and pass | ||
| Use this function to attach numpy array data to a GMT dataset and pass | ||
| it to GMT modules. Wraps ``GMT_Put_Matrix``. | ||
|
|
||
| The dataset must be created by :meth:`~gmt.clib.Session.create_data` | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.