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

chkCellMethods regex could possibly be improved. #99

Open
bzah opened this issue Oct 21, 2021 · 2 comments
Open

chkCellMethods regex could possibly be improved. #99

bzah opened this issue Oct 21, 2021 · 2 comments
Assignees
Labels

Comments

@bzah
Copy link

bzah commented Oct 21, 2021

Hello, it seemschkCellMethods initial regex (pr1) can reject some valid cell_methods.

  1. It validates only lower case methods but conventions document says:
    """Case is not significant in the method name. in chapter 7.3."""
    This is due to [a-z_] in the regex.

  2. After the name:method it expects one of the keywords ["where", "over", "within", "interval", "comment"] but I don't see in the conventions doc where it is specified that only these can be used. Beside, it tells:
    """For instance, an area-weighted mean over latitude could be indicated as lat: mean (area-weighted) or lat: mean (interval: 1 degree_north comment: area-weighted)."""
    But pr1 regex would raise an error with lat: mean (area-weighted)

Disclaimer:
I worked on a similar feature on Xclim but I never actually used cf-checker, so I might have misunderstood how chkCellMethods works. I just wanted to give you a feedback because I took some inspiration from the currentchkCellMethods.

We are now using the following regex on Xclim: (\s*\S+\s*:(\s+[\w()-]+)+)(?!\S*:) (see it in action).

Regards.

@RosalynHatcher
Copy link
Contributor

RosalynHatcher commented Oct 22, 2021

Hi,

Thanks for reporting this. It looks like (1) is missing from the conformance document so I will raise a correction and then fix the checker accordingly.

(2) I've just tried cell_methods = "lat: mean (area-weighted)" and that is perfectly valid by chkCellMethods. The keywords ["where", "over", "within", "interval", "comment"] are all optional in the regex.

Regards,
Ros.

@bzah
Copy link
Author

bzah commented Oct 22, 2021

I must have missed something when trying pr1 regex, I confirm that it actually works.
Sorry for the false alarm.

@RosalynHatcher RosalynHatcher self-assigned this Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants