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

Support passing string type numbers, geographic coordinates and datetimes #975

Merged
merged 13 commits into from
Apr 23, 2021

Conversation

seisman
Copy link
Member

@seisman seisman commented Feb 26, 2021

Description of proposed changes

This PR adds support for passing string type data to GMT via GMT_Put_Vectors. Supported string type data include:

  • numeric strings (e.g., "0.5")
  • geographic coordinates (e.g, "3:30W", "3:30N")
  • datetime strings (e.g., 2010-01-01T00:00)

This PR should address the bug/feature request in #647 and #633.

To test this branch, you need GMT dev versions (i.e., GMT>=6.2.0). You can install the latest GMT dev versions using:

conda install -c conda-forge/label/dev gmt

Example script for testing:

import pygmt

fig = pygmt.Figure()
fig.basemap(region=[-10, 10, -10, 10], projection='M10c', frame=["WSEN", "afg1"])
fig.plot(x=5.0, y=5.0, style='c0.2c', color='red')
fig.plot(x='-5.0', y='-5.0', style='c0.2c', color='blue')
fig.plot(x='5:30W', y='5:30N', style='c0.2c', color='yellow')
fig.savefig("map.png")

Expected and actual output:
image

Fixes #647.
Fixes #633.

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 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 upstream Bug or missing feature of upstream core GMT feature Brand new feature labels Feb 28, 2021
@seisman seisman added this to the 0.4.0 milestone Feb 28, 2021
@seisman
Copy link
Member Author

seisman commented Mar 1, 2021

/test-gmt-dev

@weiji14
Copy link
Member

weiji14 commented Mar 2, 2021

Can you add a doc API line at https://github.com/GenericMappingTools/pygmt/blame/v0.3.0/doc/api/index.rst#L214 for clib.Session.put_strings? Just realized we forgot to add it during #520.

@seisman
Copy link
Member Author

seisman commented Mar 2, 2021

Can you add a doc API line at v0.3.0/doc/api/index.rst#L214 (blame) for clib.Session.put_strings? Just realized we forgot to add it during #520.

Done in 24ad0af

@seisman
Copy link
Member Author

seisman commented Mar 2, 2021

See https://github.com/GenericMappingTools/pygmt/actions/runs/609196993 for the tests with GMT dev versions.

All the tests (including the new one) pass.

Tests with GMT 6.1.1 fails, because in the old codes, datetime strings can processed by pd.to_datetime, but now datetime strings are passed to GMT directly. As expected, passing datetime strings to GMT works for GMT dev but not GMT 6.1.1, so we can't merge the PR unless GMT 6.2.0 is released.

Comment on lines 117 to 118
for i, j in itertools.combinations_with_replacement(range(3), r=2):
# TODO: Change range(3) to range(4)
Copy link
Member Author

Choose a reason for hiding this comment

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

The idea of the test is to pass different combinations of strings to GMT, let GMT write it to a temporary file, and check if the output is the same as expected (i.e., if vector=expected_vetcor.

Still has trouble testing datetime strings, when the text file contains strings,

dtype=[("x", np.str_), ("y", np.str_)],

doesn't works as I expect.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in bf7587c, but could still be improved.

@seisman
Copy link
Member Author

seisman commented Mar 2, 2021

/test-gmt-dev

https://github.com/GenericMappingTools/pygmt/actions/runs/613257718

@seisman seisman changed the title WIP: Support for passing strings to GMT via GMT_Put_Vectors Support for passing strings to GMT via GMT_Put_Vectors Mar 2, 2021
@seisman seisman changed the title Support for passing strings to GMT via GMT_Put_Vectors Support passing string type number, geographic coordinates and datetimes Mar 2, 2021
@seisman seisman marked this pull request as ready for review April 23, 2021 21:43
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.

Great to finally see this come through!

@weiji14 weiji14 changed the title Support passing string type number, geographic coordinates and datetimes Support passing string type numbers, geographic coordinates and datetimes Apr 23, 2021
@seisman seisman merged commit b2fd391 into master Apr 23, 2021
@seisman seisman deleted the put-vector-text branch April 23, 2021 23:03
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…imes (GenericMappingTools#975)

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
Labels
feature Brand new feature upstream Bug or missing feature of upstream core GMT
Projects
None yet
2 participants