Fix a failing test due to GMT API change#556
Merged
Conversation
The test `test_put_matrix_grid` fails due to recent changes of GMT API in GenericMappingTools/gmt#3848. Before this PR, for a matrix grid, calling GMT_Write_Data always writes the matrix grid as a matrix, no matter the geometry is `GMT_IS_POINT` or `GMT_IS_SURFACE`. This PR declares it as a bug (or an improvement). With that PR merged, `geometry=GMT_IS_POINT` writes a matrix, `geometry=GMT_IS_SURFACE` writes a the matrix grid as a netCDF file. This test expects a matrix, but the new API writes a netCDF grid. That's why it fails. This PR fixes the issue by simply changing `GMT_IS_SURFACE` to `GMT_IS_POINT`, so that the test can pass for GMT 6.1.0, 6.1 and master branches.
weiji14
reviewed
Aug 7, 2020
| lib.write_data( | ||
| "GMT_IS_MATRIX", | ||
| "GMT_IS_SURFACE", | ||
| "GMT_IS_POINT", |
Member
There was a problem hiding this comment.
It doesn't seem like we use lib.write_data on any user-facing code, or very much in the clib. I know GMT can write an ASCII grid, but maybe we should also test with "GMT_IS_SURFACE" to ensure that:
lib.write_datacan write to a NetCDF properly; and- We can use
xarrayto read from that NetCDF file
Member
Author
There was a problem hiding this comment.
Sounds a good idea, but it needs 6.1.1, so better to add the test in a new pr and only merge that pr after we bump to gmt 6.1.1
Member
There was a problem hiding this comment.
Ah ok (forgot that we're still on GMT 6.1.0). Leave it until end of the month then.
weiji14
approved these changes
Aug 7, 2020
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description of proposed changes
The test
test_put_matrix_gridfails with GMT latest due to recent changes of GMT APIin GenericMappingTools/gmt#3848.
Before this PR, for a matrix grid, calling GMT_Write_Data always writes
the matrix grid as a matrix, no matter the geometry is
GMT_IS_POINTorGMT_IS_SURFACE.This PR declares it as a bug (or an improvement). With that PR merged,
geometry=GMT_IS_POINTwrites the matrix as a table,geometry=GMT_IS_SURFACEwrites the matrix grid as a netCDF file.
This test expects a matrix, but the new API writes a netCDF grid.
That's why it fails.
This PR fixes the issue by simply changing
GMT_IS_SURFACEtoGMT_IS_POINT, so that the test can pass for GMT 6.1.0, 6.1 and masterbranches.
Fixes #
Reminders
make formatandmake checkto make sure the code follows the style guide.doc/api/index.rst.