Skip to content

Conversation

@seisman
Copy link
Member

@seisman seisman commented Apr 3, 2024

Description of proposed changes

Address #3148 (comment).

Two main changes:

  1. Add the _GMT_DATASET.to_strings method which returns an array of the trailing text
  2. Add output_type="strings" to Session.virtualfile_to_dataset which wraps the _GMT_DATASET.to_strings method

Tests are also added. The method name .to_strings is similar to pd.DataFrame.to_string, but since we return an array of strings, we use to_strings instead of to_string.

@seisman seisman added enhancement Improving an existing feature needs review This PR has higher priority and needs review. labels Apr 3, 2024
@seisman seisman added this to the 0.12.0 milestone Apr 3, 2024
@seisman seisman force-pushed the dataset/to_strings branch from d9d2f9b to 2442fda Compare April 3, 2024 02:23
... assert result is None
... assert Path(outtmp.name).stat().st_size > 0
...
... # strings output
Copy link
Member Author

@seisman seisman Apr 3, 2024

Choose a reason for hiding this comment

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

Maybe I should move all the doctests except the pandas one into the separate test_clib_virtualfile.py file since the docstrings are too long.

Edit: Not as easy as I initially thought. So I'll keep them in the docstrings now.

@seisman seisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Apr 5, 2024
@seisman seisman requested a review from a team April 5, 2024 11:01
@seisman seisman merged commit 6193938 into main Apr 6, 2024
@seisman seisman deleted the dataset/to_strings branch April 6, 2024 08:51
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Apr 6, 2024
@seisman
Copy link
Member Author

seisman commented Apr 6, 2024

Self-approved and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improving an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants