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

Handling comments in "cell_methods" in PrePARE #587

Closed
mauzey1 opened this issue Mar 13, 2020 · 1 comment · Fixed by #591
Closed

Handling comments in "cell_methods" in PrePARE #587

mauzey1 opened this issue Mar 13, 2020 · 1 comment · Fixed by #591
Labels
Projects

Comments

@mauzey1
Copy link
Collaborator

mauzey1 commented Mar 13, 2020

When PrePARE checks the value of cell_methods in a file with that of the table, it will only check the file value before the pattern " (".

if key == "cell_methods":
idx = file_value.find(" (")
if idx != -1:
file_value = file_value[:idx]
table_value = table_value[:idx]

This is meant to skip comments in the file value. However, it will also skip anything that is after the comment. Some variables have cell_methods values that have other words proceeding what is in parentheses.

Examples:
https://github.com/PCMDI/cmip6-cmor-tables/blob/3b802b4e94fc36c5c9d1c9234fcace7d81f769c3/Tables/CMIP6_Omon.json#L1241-L1258
https://github.com/PCMDI/cmip6-cmor-tables/blob/3b802b4e94fc36c5c9d1c9234fcace7d81f769c3/Tables/CMIP6_EmonZ.json#L125-L142

If the file had a cell_methods value of "longitude: sum (comment: basin sum [along zig-zag grid path]) depth: sum time: mean", then it would only check for "longitude: sum" in the table's value.

Should we find a way to ignore everything in parentheses in cell_methods but still check for everything else in the cell_methods attribute?

@mauzey1 mauzey1 added the bug label Mar 13, 2020
@taylor13
Copy link
Collaborator

Thanks for noting this issue. Yes, it would be great to do what you say. It doesn't seem that this would be too difficult. One possible problem with the simplest approach ("when a parenthesis is found, search for an end parenthesis, and proceed to analyze the cell_methods string from there") would arise if there were embedded parentheses. I'm pretty sure we don't have any data like that, but I don't think embedded parentheses are forbidden by the convention. Would it be difficult to treat this more difficult case? If so, I would simply implement a solution to the simple case, which will be a big improvement from the current treatment.

@mauzey1 mauzey1 added this to To do in 3.6.0 via automation Mar 27, 2020
@mauzey1 mauzey1 moved this from To do to In progress in 3.6.0 Mar 27, 2020
@mauzey1 mauzey1 moved this from In progress to Done in 3.6.0 Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
3.6.0
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants