rewrote the subset selector for sgrid, added test based on example notebook #61
Conversation
| ''' | ||
| This is a basic integration test for the subsetting of a ROMS sgrid dataset using a polygon. | ||
| ''' | ||
| fs = fsspec.filesystem( |
There was a problem hiding this comment.
we need a test (or more) that doesn't rely on that server being alive ....
See PR comment.
There was a problem hiding this comment.
I added the arakawa_c_test_grid from gridded for future tests. Will add another offline test soon
|
|
||
| node_lon: xr.DataArray | None = None | ||
| node_lat: xr.DataArray | None = None | ||
| for c in node_coords: |
There was a problem hiding this comment.
hmm -- doesn't the spec say it should always be lon, lat order? -- but I suppose this is a bit safer anyway.
| elif 'lat' in c: | ||
| node_lat = ds[c] | ||
| if node_lon is None or node_lat is None: | ||
| raise ValueError(f"Could not find lon and lat for dimension {node_dims}") |
There was a problem hiding this comment.
this is specifically the node_coordinates -- yes? If so, the error message should be clear on that.
xarray_subset_grid/utils.py
Outdated
| @@ -151,3 +151,29 @@ def compute_2d_subset_mask( | |||
| polygon_mask = np.where(polygon_mask > 1, True, False) | |||
|
|
|||
| return xr.DataArray(polygon_mask, dims=mask_dims) | |||
|
|
|||
| def parse_padding_string(dim_string): | |||
There was a problem hiding this comment.
it'd be nice to have a unit test for this.
There was a problem hiding this comment.
also -- is it really this complex ? that surprises me -- though I haven't followed all the logic. If any of the logic is to deal with non-compliant files, then that should go elsewhere. maybe it's not.
tests/utilities.py
Outdated
| import pytest | ||
| import glob | ||
|
|
||
| from .get_remote_data import get_datafile |
There was a problem hiding this comment.
we don't want to put this in xarray_subset_grid.
I imagine we will put something like this in libgoods, or maybe gridded, but not here.
tests/utilities.py
Outdated
| from .get_remote_data import get_datafile | ||
|
|
||
|
|
||
| def get_test_file_dir(): |
There was a problem hiding this comment.
we can just put this in the test file directly -- or maybe make it a fixture.
And pathlib is great:
from pathlib import Path
test_file_dir = Path(__file__).parent / 'test_data')`
tests/test_grids/test_sgrid.py
Outdated
| ds_subset = ds.xsg.subset_polygon(polygon) | ||
|
|
||
| breakpoint() |
There was a problem hiding this comment.
we don't want this in there :-)
| from xarray_subset_grid.utils import get_test_file_dir | ||
|
|
||
| from gridded import Variable, VectorVariable | ||
|
|
There was a problem hiding this comment.
I tend to put this inline:
TEST_DIR = Path(__file__).parent.parent / 'test_data'
sample_sgrid_file = TEST_DIR / 'arakawa_c_test_grid.nc'
|
are the tests running / passing in your repo? Check the "actions" tab in the project page. If not -- I wonder why not? |
|
Looks good--- thanks! I'd love to see more examples and tests, and plotting, but for now this is an improvement. |
No description provided.