cmip7: restore flag-variable handling dropped by d82d1cf - #70
Closed
JanStreffing wants to merge 1 commit into
Closed
cmip7: restore flag-variable handling dropped by d82d1cf#70JanStreffing wants to merge 1 commit into
JanStreffing wants to merge 1 commit into
Conversation
Re-applies the two hunks from ESGF#49 and ESGF#60, both merged and both absent from develop, master and every release since v2.3.2. d82d1cf ("Improve and Fix Checks", 19 Jul) rewrote plugins/cmip7/cmip7.py from a base predating both PRs, so the changes were removed rather than rebased. v2.3.3 was cut two days later and its release notes credit both fixes, but neither is in the tag: v2.3.2 _is_flag_variable absent v2.3.3 _is_flag_variable absent <- release notes claim it v2.3.4 _is_flag_variable absent develop / master absent Restored, byte-identical to what was reviewed and merged: ESGF#49 variable_id fallback in the len(geo_vars) == 0 branch of _get_geo_var. Without it any file whose data variable is CF flag-valued reports "No geophysical variable detected in the file." at HIGH, which is issue ESGF#48 as originally filed. ESGF#60 _is_flag_variable helper plus the is_flag gate on the [variable.type] check and the _FillValue / missing_value attribute rules. Without it flag-valued variables are checked against data_type = "float" and constant = 1.0e20, which no integer variable can satisfy. Verified on a real CMIP7 basin fx file (AWI-ESM3-4-2-veg-HR piControl): develop geo var = None 1 HIGH "No geophysical variable detected" this geo var = 'basin' 0 findings Existence, dimensions, shape and all other attribute rules still apply. Non-flag variables are unchanged. Closes ESGF#66.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #66. This is the single restoring PR I offered there; going ahead with it since it is mechanical and only re-applies previously reviewed code. Happy to convert it to a revert of the relevant hunks instead if you prefer that shape.
d82d1cf("Improve and Fix Checks", 19 Jul) rewroteplugins/cmip7/cmip7.pyfrom a base predating #49 and #60, so both merged fixes were removed rather than rebased. v2.3.3 was cut two days later and its release notes credit both by name, but neither is actually in the tag:_is_flag_variabledevelopmasterWhat this restores
Both hunks are byte-identical to what was reviewed and merged.
#49 —
variable_idfallback in thelen(geo_vars) == 0branch of_get_geo_var. compliance-checker'sis_geophysicalexcludes anything carryingflag_meanings, which is right for QC flags but wrong for CMIP7 region selectors (basin,siline), whosestandard_nameisregionand which are the file's data variable by design. Without the fallback those files reportNo geophysical variable detected in the file.at HIGH, which is #48 exactly as originally filed.#60 — the
_is_flag_variablehelper and theis_flaggate on the[variable.type]check and the_FillValue/missing_valueattribute rules. Without it flag-valued variables are checked againstdata_type = "float"andconstant = 1.0e20, which no integer variable can satisfy.Existence, dimensions, shape and every other attribute rule still apply. Non-flag variables are unchanged.
Verification
Against a real CMIP7
basinfx file (AWI-ESM3-4-2-veg-HR piControl,int32withflag_values/flag_meanings,variable_id = "basin"):The synthetic reproducer in #59 still covers both paths.
Note
It is worth checking whether anything else merged between
5a327f0andd82d1cfwent the same way. I only looked at the two changes that affect our files.