fre.cmor: CMIP7 compatibility follow-up#730
Open
ilaflott wants to merge 33 commits intouse-cmip7-tablesv2from
Open
fre.cmor: CMIP7 compatibility follow-up#730ilaflott wants to merge 33 commits intouse-cmip7-tablesv2from
fre.cmor: CMIP7 compatibility follow-up#730ilaflott wants to merge 33 commits intouse-cmip7-tablesv2from
Conversation
ilaflott
changed the title
fre.cmor\: CMIP7 compatibility follow-upfre.cmor: CMIP7 compatibility follow-up
ceblanton
reviewed
Feb 6, 2026
…list (minus the brands), old behavior with no mip_vars arg. should be cmip6 backwards compatible actually
Add `fre cmor config`, a new CLI subtool that generates CMOR YAML configuration files for `fre cmor yaml` to consume. Refactored from the quick_script.py prototype. New `fre cmor config` feature: - fre/cmor/cmor_config.py: cmor_config_subtool() scans a pp directory tree, cross-references variables against MIP tables via make_simple_varlist(), filters out non-variable tables, and writes structured YAML output. - fre/cmor/__init__.py: export cmor_config_subtool - fre/cmor/frecmor.py: add `config` click command with 12 options - fre/cmor/cmor_finder.py: update docstring for make_simple_varlist - fre/tests/test_fre_cmor_cli.py: add 4 config tests (no-args, --help, opt-dne, end-to-end case1) - test_call.sh: developer shell script with example CLI calls - Remove quick_script.py (superseded by cmor_config module) CMIP7 run/yaml improvements: - fre/cmor/cmor_helpers.py: add find_gold_ocean_statics_file() to copy archived OM5_025 ocean_static.nc for tripolar grid handling - fre/cmor/cmor_mixer.py: use find_gold_ocean_statics_file() in rewrite_netcdf_file_var instead of find_statics_file() for CMIP7 - fre/cmor/cmor_yamler.py: route cmor_run_subtool outdir per component/table_name for organized output structure - fre/tests/test_files/CMOR_CMIP7_input_example.json: fix calendar (julian -> noleap) and grid placeholder values
…ponent, freq fallback from MIP tables - Deprecate settings.yaml from CMOR yaml combining path; cmor yamls are now self-contained (pp_dir, table_dir, outdir defined inline) - Move variable_list from table-level to component-level in cmor yaml schema, cmor_yamler.py, and test fixtures - Derive freq from CMIP6 MIP tables when not specified in cmor yaml (CMIP7 tables lack freq, so user must supply it explicitly) - Fix gold ocean statics: guard against stale directory at file path, use is_file() instead of exists(), add proper fallback to legacy find_statics_file - Monkeypatch find_gold_ocean_statics_file in Omon_sos_gn test to use local CDL-generated statics instead of /archive - Rewrite calendar integration tests for readability (add outpath to fixture, shared mock list, concise docstrings) - Update COMPARE_TEST_OUTPUT_cmor.yaml and cmor.am5.yaml to match All 99 tests pass, 1 skipped.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## use-cmip7-tablesv2 #730 +/- ##
======================================================
+ Coverage 82.65% 83.42% +0.76%
======================================================
Files 68 70 +2
Lines 4578 4807 +229
======================================================
+ Hits 3784 4010 +226
- Misses 794 797 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…rand possibilities.
…NT_CLI_CALL -> CLI option
Add 21 new tests across two test files covering previously untested functions and edge cases: test_cmor_helpers.py (19 new): - create_tmp_dir: success, with exp_config, OSError on bad path - get_json_file_data: success, nonexistent file, invalid JSON - update_grid_and_label: ValueError when grid_label/grid/nom_res is None - get_bronx_freq_from_mip_table: success (mon→monthly), invalid freq - update_outpath: ValueError when json_file_path or output is None - filter_brands: time filter (mean/inst), vertical filter, all eliminated, multiple remaining test_cmor_finder_make_simple_varlist.py (2 new): - Duplicate var_name deduplication - MIP table filtering (only vars in table survive) Also remove unreachable dead code (else branch with bare raise) at the end of cmor_find_subtool — the preceding if/elif already covers all reachable paths and a ValueError guard earlier makes the else logically impossible.
…ve_missing
The test asserted the mirrored directory tree included 'gold/' but the
function's split('/')[3:] strips the first three path segments
('', 'archive', 'gold') from '/archive/gold/datasets/...', so the
created subdirectory starts at 'datasets/'.
Locally the archive file exists, so the test took the else-branch and
never hit the bad assertion. On GitHub CI the archive is absent, the
if-branch ran, and the wrong expected path caused the failure.
ilaflott
requested review from
GFDL-Eric,
aradhakrishnanGFDL,
ceblanton and
singhd789
ilaflott
added
documentation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements CMIP7 compatibility features for the fre.cmor tool, building on previous work to support both CMIP6 and CMIP7 workflows. The changes centralize constants, add brand disambiguation for CMIP7 variables, introduce a YAML configuration generator, and significantly expand test coverage.
Changes:
- Introduces
cmor_constants.pyto centralize all hard-coded constants previously scattered across multiple modules - Adds
filter_brands()helper function for CMIP7 multi-brand variable disambiguation using time-type and vertical-coordinate signals - Implements new
fre cmor configcommand to auto-generate CMOR YAML configurations from post-processing directory trees - Expands test suite to 102 tests with comprehensive coverage of exception paths, brand filtering, and frequency derivation logic
- Deprecates settings.yaml imports for the cmor path — cmor yamls are now self-contained
- Adds
--print_cli_call/--no-print_cli_calloption to control dry-run output format
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| test_call.sh | Development/testing script with manual test cases (should not be in repo) |
| pylintrc | Raises minimum quality threshold from 8.00 to 8.30 |
| fre/yamltools tests | Updates test fixtures to reflect settings.yaml deprecation for cmor |
| fre/yamltools/combine_yamls_script.py | Removes settings.yaml merge for cmor path |
| fre/tests/test_fre_cmor_cli.py | Adds CLI tests for config and varlist commands with MIP table filtering |
| fre/tests/test_files/CMOR_CMIP7_input_example.json | Updates test data calendar and grid fields |
| fre/cmor/tests/test_cmor_yamler_subtool.py | New comprehensive test suite for yaml subtool (17 tests) |
| fre/cmor/tests/test_cmor_run_subtool*.py | Adds tests for mip_era validation and monkeypatching for CI |
| fre/cmor/tests/test_cmor_mixer_calendar_integration.py | Refactors calendar integration tests for clarity |
| fre/cmor/tests/test_cmor_helpers.py | Expands coverage for helpers including filter_brands, gold statics, frequency derivation |
| fre/cmor/tests/test_cmor_finder_make_simple_varlist.py | Adds CLI runner coverage and edge case tests |
| fre/cmor/frecmor.py | Adds config command and print_cli_call option to yaml command |
| fre/cmor/cmor_yamler.py | Implements CMIP7 freq handling, print_cli_call parameter, moves variable_list to component level |
| fre/cmor/cmor_mixer.py | Integrates filter_brands, gold ocean statics lookup, update_outpath call, improved calendar handling |
| fre/cmor/cmor_helpers.py | Adds find_gold_ocean_statics_file, update_outpath, filter_brands functions |
| fre/cmor/cmor_finder.py | Adds optional MIP table filtering to make_simple_varlist |
| fre/cmor/cmor_constants.py | New module consolidating all constants from multiple files |
| fre/cmor/cmor_config.py | New module implementing the config subtool |
| fre/cmor/init.py | Exports cmor_config_subtool |
| docs/* | Updates documentation for new features and CMIP7 support |
| .github/workflows/create_test_conda_env.yml | Removes main branch restriction from PR trigger |
Comments suppressed due to low confidence (1)
fre/cmor/cmor_yamler.py:284
- The
cmor_run_subtoolcall at line 270 is missing thecalendar_typeparameter that appears in both dry-run outputs (lines 250 and 267). This inconsistency means the actual call won't pass the calendar_type even though the dry-run shows it would be passed. Either addcalendar_type = calendar_typeto the call or remove it from the dry-run outputs.
cmor_run_subtool( #uncovered
indir = indir ,
json_var_list = json_var_list ,
json_table_config = json_mip_table_config ,
json_exp_config = json_exp_config ,
# outdir = cmorized_outdir ,
outdir = cmor_run_call_outdir ,
run_one_mode = run_one_mode ,
opt_var_name = opt_var_name ,
grid = grid_desc ,
grid_label = grid_label ,
nom_res = nom_res ,
start = start,
stop = stop
)
Member
Author
first time i found something useful in the "low-confidence" section |
…tespace here and there.
…other files- chasing down a bug seen in CI with no local equivalent, from what i can currently tell
…t the many tools which name their temp dirs, "tmp" so, new name, CMOR_tmp
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
2 participants
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.
Describe your changes
companion to gitlab.gfdl ESM45xml MR19
Core changes
cmor_constants.py— all hard-coded constants previously scattered acrosscmor_mixer,cmor_helpers,cmor_config,cmor_finder, andcmor_yamlernow live in one transparent location underfre/cmor/filter_brands()tocmor_helpersfor CMIP7 multi-brand disambiguation. when multiple MIP-table brands match a variable by dimension count, the filter narrows down to one using time-type (timevstime1viatime_bndspresence) and vertical-coordinate (INPUT_TO_MIP_VERT_DIMmapping) signals from the input datacmor_mixerbrand selection block is now a 4-line call tofilter_brands()instead of inline logicPRINT_CLI_CALLis no longer a module-level constant — it is now--print_cli_call / --no-print_cli_callonfre cmor yaml, threaded through tocmor_yaml_subtool()as aboolparameter (defaults toTrue, same behaviour as before)Test coverage improvements (102 tests across fre/cmor/tests/)
test_cmor_yamler_subtool.py(17 tests): end-to-end dry-run and non-dry-run ofcmor_yaml_subtool, plus every documented exception path — missing yamlfile, pp_dir, table_dir, exp_json, MIP table file; CMIP7 freq=None; CMIP6 freq=None with and without derivable MIP table frequency;get_bronx_freq_from_mip_tableKeyError/TypeError caught; gridding dict with None values; outdir auto-creation and creation-failure OSError; start/stop/calendar_type missing from yaml; dry-run CLI and Python call printingtest_cmor_run_subtool.pyadditions: missingmip_erain exp config raises KeyError; unsupportedmip_era(e.g. CMIP99) raises ValueErrortest_cmor_helpers.py: coverage forfind_gold_ocean_statics,check_path_existence,iso_to_bronx_chunk,conv_mip_to_bronx_freq,get_bronx_freq_from_mip_table,update_calendar_type,update_grid_and_labeltest_cmor_finder_make_simple_varlist.py: CLI runner coverage forfre cmor varlisttest_cmor_helpers.pyfor CI (archive-absent path assertion)Backwards compatibility
Issue ticket number and link (if applicable)
Closes #639
#641, #638
Checklist before requesting a review