-
Notifications
You must be signed in to change notification settings - Fork 225
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
pygmt.x2sys_cross: Refactor to use virtualfiles for output tables [BREAKING CHANGE: Dummy times in 3rd and 4th columns now have np.timedelta64 type] #3182
Merged
Merged
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
95fab98
pygmt.x2sys_cross: Add 'output_type' parameter for output in pandas/n…
seisman ce926a0
Move session-unrelated code block outside the session block
seisman 86278cb
Refactor if-else using match statements
seisman 58c6ea4
Fix static typing issue
seisman d6eeade
Fix warnings
seisman 9d12ae1
Convert dummpy times to timedelta
seisman 28eb1df
Let validate_output_table_type specify the supported output types
seisman 5e926e8
Fix
seisman c1c756d
Merge branch 'main' into validator/valid-types
seisman 3a3df0a
Update docstrings
seisman 3aea9a6
Merge branch 'main' into validator/valid-types
seisman d869a32
Merge branch 'main' into validator/valid-types
seisman b46d21d
Remove support of numpy output type
seisman 81a1ec0
Merge branch 'validator/valid-types' into refactor/x2sys_cross
seisman 07fe53e
Merge branch 'main' into refactor/x2sys_cross
seisman 5f04506
Remove the output_type parameter
seisman 84765e4
Remove the output_type parameter from tests
seisman b9b4098
Improve tests
seisman bf59e61
Merge branch 'main' into refactor/x2sys_cross
seisman 9bc063a
Deal with TIME_EPOCH
seisman b0b5099
Support TIME_UNIT for 'd' and 's'
seisman 1396ee8
Fix a typo
seisman 6f2671a
Update
seisman a9a4179
Fix
seisman 04a1986
Typo
seisman b27212b
Fix
seisman aa3e9af
Merge branch 'main' into refactor/x2sys_cross
seisman 97312fb
Improve the handling of TIME_UNIT
seisman 6450ba0
Call the apply function separately
seisman 29a7f9e
Merge branch 'main' into refactor/x2sys_cross
seisman d5294a4
Further simplify the logic
seisman 55f7c30
Fix
seisman a44390d
Further simplification
seisman b81e292
Improve tests
seisman 870d9c7
Update pygmt/src/x2sys_cross.py
seisman db94b91
Fix x2sys_cross tests on macOS M
seisman de17d5e
Update pygmt/src/x2sys_cross.py
seisman ebce56e
Update pygmt/src/x2sys_cross.py
seisman 71af717
Merge branch 'main' into refactor/x2sys_cross
seisman cf2cfc7
Merge branch 'main' into refactor/x2sys_cross
seisman 3a62fc1
Merge branch 'main' into refactor/x2sys_cross
seisman 9fd35ce
Apply suggestions from code review
seisman 2b3474b
Update pygmt/tests/test_x2sys_cross.py
seisman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that
x2sys_cross
can output multi-segment files (each segment is separated byIO_SEGMENT_MARKER
which is>
by default, see https://docs.generic-mapping-tools.org/6.5/reference/file-formats.html#optional-segment-header-records). If I'm not mistaken, the currentvirtualfile_to_dataset
method does not implement multi-segment file handling yet? To be fair though, the current implementation inx2sys_cross
simply merges all segments into one, since we skip rows starting with>
, but we need to check thatvirtualfile_to_dataset
will return all segments in a multi-segment file instead of just the first one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The main problem is that, as far as I know, there is no equivalent way to represent a multi-segment file in pandas. The multi-segment support was also mentioned in #2729 (comment).
If we can have a general way to represent multi-segment in pandas, then it should be straightforward to output multi-segments from
_GMT_DATASET
to the desired data structure.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already tested in the
_GMT_DATASET
docstringspygmt/pygmt/datatypes/dataset.py
Line 215 in 466c8b6
For
x2sys_cross
, we also test the shape of the output pd.DataFrame.