Skip to content
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.

Sign up for GitHub

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

Metadata Improvements #106

Merged
merged 4 commits into from
Dec 16, 2024
Merged

Metadata Improvements #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
708d8ef
add crs to GetMetadata requests
ndellicarpini Dec 11, 2024
d8218d9
check for coord in variables for additional_coords
ndellicarpini Dec 11, 2024
043b489
use default values in GetMap for additional coords not specified in r…
ndellicarpini Dec 12, 2024
1102b99
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions xpublish_wms/query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ class WMSGetMetadataQuery(WMSBaseQuery):
description="Bounding box to use for calculating min and max in the format 'minx,miny,maxx,maxy'",
),
)
crs: Literal["EPSG:4326", "EPSG:3857"] = Field(
"EPSG:4326",
description="Coordinate reference system to use for the query. EPSG:4326 and EPSG:3857 are supported for this request",
)
time: Optional[str] = (
Field(
None,
Expand Down Expand Up @@ -273,6 +277,7 @@ def wms_query(
day=day,
range=range,
bbox=bbox,
crs=crs if srs is None else srs,
time=time,
elevation=elevation,
)
Expand Down
38 changes: 26 additions & 12 deletions xpublish_wms/wms/get_map.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ def ensure_query_types(
self.autoscale = query.autoscale

available_selectors = ds.gridded.additional_coords(ds[self.parameter])
self.dim_selectors = {k: query_params[k] for k in available_selectors}
self.dim_selectors = {
k: query_params[k] if k in query_params else None
for k in available_selectors
}

def select_layer(self, ds: xr.Dataset) -> xr.DataArray:
"""
Expand Down Expand Up @@ -235,23 +238,34 @@ def select_elevation(self, ds: xr.Dataset, da: xr.DataArray) -> xr.DataArray:
def select_custom_dim(self, da: xr.DataArray) -> xr.DataArray:
"""
Select other dimension, ensuring a 2D array

If dimension is provided :
- use xarray to access custom coord
- method nearest to ensure at least one result

Otherwise:
- Get first value of coord

:param da:
:return:
"""
# Filter dimension from custom query, if any
for dim, value in self.dim_selectors.items():
if dim in da.coords:
dtype = da[dim].dtype
method = None
if "timedelta" in str(dtype):
value = pd.to_timedelta(value)
elif np.issubdtype(dtype, np.integer):
value = int(value)
method = "nearest"
elif np.issubdtype(dtype, np.floating):
value = float(value)
method = "nearest"
da = da.sel({dim: value}, method=method)
if value is None:
da = da.isel({dim: 0})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing this is necessary because otherwise you get an error if you dont specify?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well the actual issue was happening at L177 where it would try to get a query parameter that could sometimes be missing (as it seems like we allow any extent to be missing from the query), so that's why I added the if k in query_params else None there. And since I would imagine it wouldn't work to try and get the closest value to None, I added that condition to just get the first value.

Copy link
Collaborator

@mpiannucci mpiannucci Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooook got it. Ok so i think L177 might work as this instead? This would filter instead of having to set them as None

{ k: query_params[k] for k in available_selectors if k in query_params}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would work except that we would need to use ds.gridded.additional_coords(ds[self.parameter]) to iterate through the extra dimensions rather than self.dim_selectors, which I'm not sure about. We would always need to filter the data by those dims in order for to have the correct number of dims for GetMap right?

Copy link
Collaborator

@mpiannucci mpiannucci Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah its true. Its a decision about whether a tile should work by default vs if it should fail if the user didnt pass in dimensions.

OKay I like your way, it matches the get caps too!

else:
dtype = da[dim].dtype
method = None
if "timedelta" in str(dtype):
value = pd.to_timedelta(value)
elif np.issubdtype(dtype, np.integer):
value = int(value)
method = "nearest"
elif np.issubdtype(dtype, np.floating):
value = float(value)
method = "nearest"
da = da.sel({dim: value}, method=method)

# Squeeze single value dimensions
da = da.squeeze()
Expand Down
7 changes: 5 additions & 2 deletions xpublish_wms/wms/get_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def get_minmax(
bbox=query.bbox if not entire_layer else "-180,-90,180,90",
width=1 if entire_layer else 512,
height=1 if entire_layer else 512,
crs="EPSG:4326",
crs=query.crs,
time=query.time,
elevation=query.elevation,
styles="raster/default",
Expand Down Expand Up @@ -139,7 +139,10 @@ def get_layer_details(ds: xr.Dataset, layer_name: str) -> dict:

additional_coords = ds.gridded.additional_coords(da)
additional_coord_values = {
coord: da.cf.coords[coord].values.tolist() for coord in additional_coords
coord: (
da.cf.coords[coord] if coord in da.cf.coords else da[coord]
).values.tolist()
for coord in additional_coords
}

return {
Expand Down
Loading