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.

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

merged 4 commits into from
Dec 16, 2024

Conversation

ndellicarpini
Copy link
Collaborator

  • added SRS/CRS to GetMetadata requests
  • added default values for extra dimensions (non time/elevation) when requesting GetMap without specifying dimension values in the query params
  • fix extra dimension support when dimensions are not CF compliant.

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

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

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!

@mpiannucci mpiannucci merged commit 112696a into main Dec 16, 2024
5 checks passed
@mpiannucci mpiannucci deleted the getmetadata-crs branch December 16, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants