Skip to content

Support passing data in numpy int8, int16, uint8 and uint16 dtypes to GMT#1963

Merged
seisman merged 4 commits intomainfrom
add-datatypes
Jun 16, 2022
Merged

Support passing data in numpy int8, int16, uint8 and uint16 dtypes to GMT#1963
seisman merged 4 commits intomainfrom
add-datatypes

Conversation

@seisman
Copy link
Member

@seisman seisman commented Jun 15, 2022 •
edited
Loading

Description of proposed changes

Support numpy data types np.int8, np.int16, np.uint8, and np.uint16.

References:

Fixes #1960.

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 wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • 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

@seisman seisman added this to the 0.7.0 milestone Jun 15, 2022
@seisman seisman mentioned this pull request Jun 15, 2022
Closed
@seisman seisman added the enhancement Improving an existing feature label Jun 15, 2022
@seisman seisman changed the title Support passing more data types to GMT Support passing more numpy dtypes to GMT Jun 15, 2022
@weiji14
Copy link
Member

weiji14 commented Jun 15, 2022

Awesome @seisman! Didn't realize GMT even supported these other dtypes!

Just need to update some tests, e.g. these lines:

pygmt/pygmt/tests/test_clib_put_matrix.py

Line 18 in 1067fa3

dtypes = "float32 float64 int32 int64 uint32 uint64".split()

pygmt/pygmt/tests/test_clib_put_matrix.py

Line 64 in 1067fa3

dtypes = "float32 float64 int32 int64 uint32 uint64".split()

pygmt/pygmt/tests/test_clib_put_vector.py

Line 19 in 1067fa3

dtypes = "float32 float64 int32 int64 uint32 uint64".split()

pygmt/pygmt/tests/test_clib_put_vector.py

Line 59 in 1067fa3

dtypes = "float32 float64 int32 int64 uint32 uint64".split()

Also 6 more lines in test_clib.py. Maybe a good idea to make the dtypes list into a pytest fixture, use something like:

Use dtypes = "int8 int16 int32 int64 uint8 uint16 uint32 uint64 float32 float64".split()

@seisman
Copy link
Member Author

seisman commented Jun 15, 2022

Awesome @seisman! Didn't realize GMT even supported these other dtypes!

Just need to update some tests, e.g. these lines:

pygmt/pygmt/tests/test_clib_put_matrix.py

Line 18 in 1067fa3

dtypes = "float32 float64 int32 int64 uint32 uint64".split()

pygmt/pygmt/tests/test_clib_put_matrix.py

Line 64 in 1067fa3

dtypes = "float32 float64 int32 int64 uint32 uint64".split()

pygmt/pygmt/tests/test_clib_put_vector.py

Line 19 in 1067fa3

dtypes = "float32 float64 int32 int64 uint32 uint64".split()

pygmt/pygmt/tests/test_clib_put_vector.py

Line 59 in 1067fa3

dtypes = "float32 float64 int32 int64 uint32 uint64".split()

Also 6 more lines in test_clib.py. Maybe a good idea to make the dtypes list into a pytest fixture, use something like:

Use dtypes = "int8 int16 int32 int64 uint8 uint16 uint32 uint64 float32 float64".split()

Was working on it. Now pushed to this branch.

seisman
seisman commented Jun 15, 2022
View reviewed changes
pygmt/clib/session.py Outdated
Comment on lines +863 to +864
Not at all numpy dtypes are supported, only: int8, int16, int32, int64,
uint8, uint16, uint32, uint64, float32, float64, str\_ and datetime64.
Copy link
Member Author

@seisman seisman Jun 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that passing str_ type matrix to GMT is allowed in the PyGMT put_matrix function, but was not documented here. But I'm wondering what happens if passing a str matrix to GMT.

Copy link
Member

@weiji14 weiji14 Jun 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Not at all numpy dtypes are supported, only: int8, int16, int32, int64,
uint8, uint16, uint32, uint64, float32, float64, str\_ and datetime64.
Not all numpy dtypes are supported, only: int8, int16, int32, int64,
uint8, uint16, uint32, uint64, float32, float64, str\_ and datetime64.

I just realized that passing str_ type matrix to GMT is allowed in the PyGMT put_matrix function, but was not documented here. But I'm wondering what happens if passing a str matrix to GMT.

Maybe try and see?

seisman
seisman commented Jun 15, 2022
View reviewed changes
weiji14
weiji14 reviewed Jun 15, 2022
View reviewed changes
pygmt/clib/session.py Outdated
Comment on lines +863 to +864
Not at all numpy dtypes are supported, only: int8, int16, int32, int64,
uint8, uint16, uint32, uint64, float32, float64, str\_ and datetime64.
Copy link
Member

@weiji14 weiji14 Jun 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Not at all numpy dtypes are supported, only: int8, int16, int32, int64,
uint8, uint16, uint32, uint64, float32, float64, str\_ and datetime64.
Not all numpy dtypes are supported, only: int8, int16, int32, int64,
uint8, uint16, uint32, uint64, float32, float64, str\_ and datetime64.

I just realized that passing str_ type matrix to GMT is allowed in the PyGMT put_matrix function, but was not documented here. But I'm wondering what happens if passing a str matrix to GMT.

Maybe try and see?

@weiji14
Copy link
Member

weiji14 commented Jun 15, 2022 •
edited
Loading

Just as an aside, is there any chance of GMT supporting half precision floats? I.e. https://numpy.org/doc/stable/reference/arrays.scalars.html#numpy.float16, or 2-byte floats. Not sure if it's possible in C, but supporting float16 will be useful for some machine learning workloads that use float16.

@seisman
Copy link
Member Author

seisman commented Jun 15, 2022

Looking at the source code of the GMT_Put_Matrix function (https://github.com/GenericMappingTools/gmt/blob/b552be3a3f76455cd45f32202d468d14ce93cd28/src/gmt_api.c#L15458) and the gmtapi_insert_vector function (https://github.com/GenericMappingTools/gmt/blob/b552be3a3f76455cd45f32202d468d14ce93cd28/src/gmt_api.c#L15297), I think only numeric data types are supported.

@seisman @weiji14
Fix typos
603efb5 
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
weiji14
weiji14 approved these changes Jun 15, 2022
View reviewed changes
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me, but maybe wait and see if @De-Guo has time to verify that it fixes his issue at #1960.

@weiji14 weiji14 added the final review call This PR requires final review and approval from a second reviewer label Jun 15, 2022
@seisman seisman marked this pull request as ready for review June 15, 2022 16:10
@seisman
Copy link
Member Author

seisman commented Jun 15, 2022

Just as an aside, is there any chance of GMT supporting half precision floats? I.e. numpy.org/doc/stable/reference/arrays.scalars.html#numpy.float16, or 2-byte floats. Not sure if it's possible in C, but supporting float16 will be useful for some machine learning workloads that use float16.

I have no idea about float16. Better to open a feature request in the GMT repository.

@seisman seisman changed the title Support passing more numpy dtypes to GMT Support passing data in numpy int8, int16, uint8 and uint16 dtypes to GMT Jun 15, 2022
@seisman seisman merged commit 9f9b7f6 into main Jun 16, 2022
@seisman seisman deleted the add-datatypes branch June 16, 2022 10:51
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Jun 16, 2022
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
@seisman @weiji14
Support passing data in numpy int8, int16, uint8 and uint16 dtypes to…
e113af9 
… GMT (GenericMappingTools#1963)

Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@weiji14 weiji14 weiji14 approved these changes

Assignees

No one assigned

Labels

enhancement Improving an existing feature

Projects

None yet

Milestone

0.7.0

Development

Successfully merging this pull request may close these issues.

uint8 datatype issue

2 participants

@seisman @weiji14