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

add check for Sub_conventions for CFradial #4915

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chrishavlin
Copy link
Contributor

This PR adjusts the cf_radial frontend _is_valid to look in an additional attribute (Sub_conventions).

NEXRAD (https://cmr.earthdata.nasa.gov/search/concepts/C2020894988-GHRC_DAAC.html) uses CFradial but has slightly different metadata from what we're checking for. In this case it's got the following attributes:

Conventions :
    CF-1.7
Sub_conventions :
    CF-Radial instrument_parameters radar_parameters radar_calibration

@chrishavlin chrishavlin added enhancement Making something better code frontends Things related to specific frontends labels May 30, 2024
if hasattr(ds, c):
cons += getattr(ds, c)
is_cfrad = "CF/Radial" in cons
is_cfrad = "CF/Radial" in cons or "CF-Radial" in cons
Copy link
Member

Choose a reason for hiding this comment

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

while your patch seems uncontroversial, I'm having trouble understand the existing implementation: why is cons constructed as a concatenated str instead of a list[str] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll double check but I think it's because those attribute values are space-delimted strings. e.g., "CF-Radial instrument_parameters radar_parameters radar_calibration" is actually 'CF-Radial instrument_parameters radar_parameters radar_calibration' so you have to search for a substring. I'll confirm that and add a comment in the code here (or update the code if I'm wrong).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK: confirmed just now, and pushed up a comment so that it's clearer in the future

@neutrinoceros neutrinoceros added this to the 4.4.0 milestone May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code frontends Things related to specific frontends enhancement Making something better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants