-
Notifications
You must be signed in to change notification settings - Fork 229
Refactor the _load_remote_dataset function to load tiled and non-tiled grids in a consistent way #3120
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
Refactor the _load_remote_dataset function to load tiled and non-tiled grids in a consistent way #3120
Changes from all commits
004f985
baacd32
6f75d9f
2941633
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,12 +4,12 @@ | |
|
||
from __future__ import annotations | ||
|
||
from typing import TYPE_CHECKING, ClassVar, NamedTuple | ||
from typing import TYPE_CHECKING, ClassVar, Literal, NamedTuple | ||
|
||
from pygmt.clib import Session | ||
from pygmt.exceptions import GMTInvalidInput | ||
from pygmt.helpers import kwargs_to_strings | ||
from pygmt.io import load_dataarray | ||
from pygmt.src import grdcut, which | ||
from pygmt.helpers import build_arg_list, kwargs_to_strings | ||
from pygmt.src import which | ||
|
||
if TYPE_CHECKING: | ||
import xarray as xr | ||
|
@@ -344,7 +344,7 @@ def _load_remote_dataset( | |
dataset_prefix: str, | ||
resolution: str, | ||
region: str | list, | ||
registration: str, | ||
registration: Literal["gridline", "pixel", None], | ||
) -> xr.DataArray: | ||
r""" | ||
Load GMT remote datasets. | ||
|
@@ -370,54 +370,68 @@ def _load_remote_dataset( | |
|
||
Returns | ||
------- | ||
grid : :class:`xarray.DataArray` | ||
grid | ||
The GMT remote dataset grid. | ||
|
||
Note | ||
---- | ||
The returned :class:`xarray.DataArray` doesn't support slice operation for tiled | ||
grids. | ||
The registration and coordinate system type of the returned | ||
:class:`xarray.DataArray` grid can be accessed via the GMT accessors (i.e., | ||
``grid.gmt.registration`` and ``grid.gmt.gtype`` respectively). However, these | ||
properties may be lost after specific grid operations (such as slicing) and will | ||
need to be manually set before passing the grid to any PyGMT data processing or | ||
plotting functions. Refer to :class:`pygmt.GMTDataArrayAccessor` for detailed | ||
explanations and workarounds. | ||
""" | ||
dataset = datasets[dataset_name] | ||
|
||
# Check resolution | ||
if resolution not in dataset.resolutions: | ||
raise GMTInvalidInput( | ||
f"Invalid resolution '{resolution}' for {dataset.title} dataset. " | ||
f"Available resolutions are: {', '.join(dataset.resolutions)}." | ||
) | ||
resinfo = dataset.resolutions[resolution] | ||
|
||
# check registration | ||
valid_registrations = dataset.resolutions[resolution].registrations | ||
# Check registration | ||
if registration is None: | ||
# use gridline registration unless only pixel registration is available | ||
registration = "gridline" if "gridline" in valid_registrations else "pixel" | ||
# Use gridline registration unless only pixel registration is available | ||
registration = "gridline" if "gridline" in resinfo.registrations else "pixel" | ||
elif registration in ("pixel", "gridline"): | ||
if registration not in valid_registrations: | ||
if registration not in resinfo.registrations: | ||
raise GMTInvalidInput( | ||
f"{registration} registration is not available for the " | ||
f"{resolution} {dataset.title} dataset. Only " | ||
f"{valid_registrations[0]} registration is available." | ||
f"{resinfo.registrations[0]} registration is available." | ||
) | ||
else: | ||
raise GMTInvalidInput( | ||
f"Invalid grid registration: '{registration}', should be either 'pixel', " | ||
"'gridline' or None. Default is None, where a gridline-registered grid is " | ||
"returned unless only the pixel-registered grid is available." | ||
) | ||
reg = f"_{registration[0]}" | ||
|
||
# different ways to load tiled and non-tiled grids. | ||
# Known issue: tiled grids don't support slice operation | ||
# See https://github.com/GenericMappingTools/pygmt/issues/524 | ||
if region is None: | ||
if dataset.resolutions[resolution].tiled: | ||
raise GMTInvalidInput( | ||
f"'region' is required for {dataset.title} resolution '{resolution}'." | ||
fname = f"@{dataset_prefix}{resolution}_{registration[0]}" | ||
if resinfo.tiled and region is None: | ||
raise GMTInvalidInput( | ||
f"'region' is required for {dataset.title} resolution '{resolution}'." | ||
) | ||
|
||
# Currently, only grids are supported. Will support images in the future. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please note that: currently we use To support images, we need to know whether the remote dataset is a grid or an image. It means we need to add the data |
||
kwdict = {"T": "g", "R": region} # region can be None | ||
with Session() as lib: | ||
with lib.virtualfile_out(kind="grid") as voutgrd: | ||
lib.call_module( | ||
module="read", | ||
args=[fname, voutgrd, *build_arg_list(kwdict)], | ||
) | ||
fname = which(f"@{dataset_prefix}{resolution}{reg}", download="a") | ||
grid = load_dataarray(fname, engine="netcdf4") | ||
else: | ||
grid = grdcut(f"@{dataset_prefix}{resolution}{reg}", region=region) | ||
grid = lib.virtualfile_to_raster(outgrid=None, vfname=voutgrd) | ||
|
||
# Full path to the grid if not tiled grids. | ||
source = which(fname, download="a") if not resinfo.tiled else None | ||
# Manually add source to xarray.DataArray encoding to make the GMT accessors work. | ||
if source: | ||
grid.encoding["source"] = source | ||
Comment on lines
+430
to
+434
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, the GMT accessors require the |
||
|
||
# Add some metadata to the grid | ||
grid.name = dataset.name | ||
|
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 also propose to
dataset_name
toname
dataset_prefix
toprefix
_
fromdataset_prefix
, e.g.,dataset_prefix="earth_relief_"
should beprefix="earth_relief"
.Please leave your comments before I make the changes.
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 feel it's better to do it in a separate PR to make this PR small for review.
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.
Issue opened at #3190.