-
Notifications
You must be signed in to change notification settings - Fork 226
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
Wrap the GMT API function GMT_Read_Data to read data into GMT data containers #3324
Merged
+239
−2
Merged
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
5a56ba8
Wrap the GMT API function GMT_Read_Data to read data into GMT data co…
seisman 7ccf641
Add tests for reading datasets and grids
seisman 024728d
Add the new API function to docs
seisman afd8dc7
Refactor to make the function definition more Pythonic
seisman 8dfb24a
Let the function return the pointer to the actual data containers, so…
seisman 76a511e
No need to call _parse_constant
seisman 591172b
Simplify the tests by converting GMT data containers into dataframe o…
seisman 3d8ccb6
Further simplify the tests
seisman d1c6249
Simple checks if rioxarray is not installed
seisman b4b034c
Let mode default to 'GMT_READ_NORMAL'
seisman 5c036fa
Rename the parameter from wesn to region
seisman 6144cfd
Merge branch 'main' into gmt-read-data
seisman 8f8494c
Improve docstrings
seisman 6206cdb
Add one more test to catch exceptions
seisman fad4cce
Fix a comment
seisman d65f7f9
Merge branch 'main' into gmt-read-data
seisman bc3494e
Allow setting family and geometry manually
seisman 9a6628d
Fix.typos
seisman 5bf40f7
Fix a typo
seisman 85fad50
Add link to the upstream documentation
seisman 3a63040
Explicitly set engine to rasterio
seisman b3cb4bc
Merge branch 'main' into gmt-read-data
seisman 8009581
Fix styling issues
seisman cc8ed2b
Explain GMT_READ_NORMAL in docstrings
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
""" | ||
Test the Session.read_data method. | ||
""" | ||
|
||
from pathlib import Path | ||
|
||
import pandas as pd | ||
import pytest | ||
import xarray as xr | ||
from pygmt.clib import Session | ||
from pygmt.exceptions import GMTCLibError | ||
from pygmt.helpers import GMTTempFile | ||
from pygmt.io import load_dataarray | ||
from pygmt.src import which | ||
|
||
try: | ||
import rioxarray # noqa: F401 | ||
|
||
_HAS_RIOXARRAY = True | ||
except ImportError: | ||
_HAS_RIOXARRAY = False | ||
|
||
|
||
@pytest.fixture(scope="module", name="expected_xrgrid") | ||
def fixture_expected_xrgrid(): | ||
""" | ||
The expected xr.DataArray object for the static_earth_relief.nc file. | ||
""" | ||
return load_dataarray(which("@static_earth_relief.nc")) | ||
|
||
|
||
def test_clib_read_data_dataset(): | ||
""" | ||
Test the Session.read_data method for datasets. | ||
""" | ||
with GMTTempFile(suffix=".txt") as tmpfile: | ||
# Prepare the sample data file | ||
with Path(tmpfile.name).open(mode="w", encoding="utf-8") as fp: | ||
print("# x y z name", file=fp) | ||
print(">", file=fp) | ||
print("1.0 2.0 3.0 TEXT1 TEXT23", file=fp) | ||
print("4.0 5.0 6.0 TEXT4 TEXT567", file=fp) | ||
print(">", file=fp) | ||
print("7.0 8.0 9.0 TEXT8 TEXT90", file=fp) | ||
print("10.0 11.0 12.0 TEXT123 TEXT456789", file=fp) | ||
|
||
with Session() as lib: | ||
ds = lib.read_data(tmpfile.name, kind="dataset").contents | ||
df = ds.to_dataframe(header=0) | ||
expected_df = pd.DataFrame( | ||
data={ | ||
"x": [1.0, 4.0, 7.0, 10.0], | ||
"y": [2.0, 5.0, 8.0, 11.0], | ||
"z": [3.0, 6.0, 9.0, 12.0], | ||
"name": pd.Series( | ||
[ | ||
"TEXT1 TEXT23", | ||
"TEXT4 TEXT567", | ||
"TEXT8 TEXT90", | ||
"TEXT123 TEXT456789", | ||
], | ||
dtype=pd.StringDtype(), | ||
), | ||
} | ||
) | ||
pd.testing.assert_frame_equal(df, expected_df) | ||
|
||
|
||
def test_clib_read_data_grid(expected_xrgrid): | ||
""" | ||
Test the Session.read_data method for grids. | ||
""" | ||
with Session() as lib: | ||
grid = lib.read_data("@static_earth_relief.nc", kind="grid").contents | ||
xrgrid = grid.to_dataarray() | ||
xr.testing.assert_equal(xrgrid, expected_xrgrid) | ||
assert grid.header.contents.n_bands == 1 # Explicitly check n_bands | ||
|
||
|
||
def test_clib_read_data_grid_two_steps(expected_xrgrid): | ||
""" | ||
Test the Session.read_data method for grids in two steps, first reading the header | ||
and then the data. | ||
""" | ||
infile = "@static_earth_relief.nc" | ||
with Session() as lib: | ||
# Read the header first | ||
data_ptr = lib.read_data(infile, kind="grid", mode="GMT_CONTAINER_ONLY") | ||
grid = data_ptr.contents | ||
header = grid.header.contents | ||
assert header.n_rows == 14 | ||
assert header.n_columns == 8 | ||
assert header.wesn[:] == [-55.0, -47.0, -24.0, -10.0] | ||
assert header.z_min == 190.0 | ||
assert header.z_max == 981.0 | ||
assert header.n_bands == 1 # Explicitly check n_bands | ||
assert not grid.data # The data is not read yet | ||
|
||
# Read the data | ||
lib.read_data(infile, kind="grid", mode="GMT_DATA_ONLY", data=data_ptr) | ||
xrgrid = data_ptr.contents.to_dataarray() | ||
xr.testing.assert_equal(xrgrid, expected_xrgrid) | ||
|
||
|
||
def test_clib_read_data_grid_actual_image(): | ||
""" | ||
Test the Session.read_data method for grid, but actually the file is an image. | ||
""" | ||
with Session() as lib: | ||
data_ptr = lib.read_data( | ||
"@earth_day_01d_p", kind="grid", mode="GMT_CONTAINER_AND_DATA" | ||
) | ||
image = data_ptr.contents | ||
header = image.header.contents | ||
assert header.n_rows == 180 | ||
assert header.n_columns == 360 | ||
assert header.wesn[:] == [-180.0, 180.0, -90.0, 90.0] | ||
# Explicitly check n_bands. Only one band is read for 3-band images. | ||
assert header.n_bands == 1 | ||
|
||
if _HAS_RIOXARRAY: # Full check if rioxarray is installed. | ||
xrimage = image.to_dataarray() | ||
expected_xrimage = xr.open_dataarray(which("@earth_day_01d_p"), engine="rasterio") | ||
assert expected_xrimage.band.size == 3 # 3-band image. | ||
xr.testing.assert_equal( | ||
xrimage, | ||
expected_xrimage.isel(band=0) | ||
.drop_vars(["band", "spatial_ref"]) | ||
.sortby("y"), | ||
) | ||
|
||
|
||
def test_clib_read_data_fails(): | ||
""" | ||
Test that the Session.read_data method raises an exception if there are errors. | ||
""" | ||
with Session() as lib: | ||
with pytest.raises(GMTCLibError): | ||
lib.read_data("not-exsits.txt", kind="dataset") |
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.
Thinking on whether to make this an enum or use
Literal
instead of a str. Looking at https://docs.generic-mapping-tools.org/6.5/devdocs/api.html#import-from-a-file-stream-or-handle, it seems like it's possible to pass inmode
as one of the variants ofGMT_enum_container
orGMT_enum_gridio
? It also looks likemode
can be None (or unset) if reading from text or table.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.
I believe the documentation is incomplete, and
mode
can accept specific values plus some modifiers, making it impossible to list all of them.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.
Yeah that what I thought too, there might be other options we's unaware of. That said, should we allow for
mode=None
(or whatever works when we don't want to set amode
)?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.
In the GMT C API,
mode
has anunsigned int
type, so themode
argument must be an integer number and can't beNone
. The C enumGMT_READ_NORMAL
has a value of0
(https://github.com/GenericMappingTools/gmt/blob/3e9fcb1f54e4e93fc1e309e0be6e2288eb79f3d2/src/gmt_resources.h#L244), which is also the default read mode.We can define
mode=None
, and add one line code like:but I feel it's not as good as setting
GMT_READ_NORMAL
as the default.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.
Ah ok then, then the
GMT_READ_NORMAL
default here makes sense. Maybe good to mention it in the docstring.