Skip to content

fre.cmor: a CMIP7 example, toward general CMIP7 compatibility #715

Open
ilaflott wants to merge 25 commits intomainfrom
use-cmip7-tablesv2
Open

fre.cmor: a CMIP7 example, toward general CMIP7 compatibility #715
ilaflott wants to merge 25 commits intomainfrom
use-cmip7-tablesv2

Conversation

@ilaflott
Copy link
Member

@ilaflott ilaflott commented Feb 2, 2026

Describe your changes

  • adds cmip7-cmor-tables as a git submodule, right next to cmip6-cmor-tables
  • adds a CMIP7-flavored example to fre/tests/test_fre_cmor_cli.py
  • remove xfail from tests that now pass (in an intended manner)
  • adds a new example (imperfect) user-input configuration for CMIP7
  • cmor_mixer is still backwards-compatible for CMIP6 cases as of this PR. for CMIP7 cases:
    • adds global GLOBAL_MIP_ERA to cmor_mixer to track which mip_era a cmorization call is for
    • adds ability to detect mip_era. if CMIP6, function as previous versions did
    • if CMIP7, remove (but track) the brand from variable name in table and move on as normal
    • if CMIP7 and multiple brands possible for one variable, track all brands
    • if CMIP7 and multiple possible brands, check requested dimensions against available dimensions in input file(s)

Issue ticket number and link (if applicable)

#641, #639

Checklist before requesting a review

  • I ran my code
  • I tried to make my code readable
  • I tried to comment my code
  • I wrote a new test, if applicable
  • I wrote new instructions/documentation, if applicable
  • I ran pytest and inspected it's output
  • I ran pylint and attempted to implement some of it's feedback
  • No print statements; all user-facing info uses logging module

@codecov
Copy link

codecov bot commented Feb 2, 2026

Codecov Report

❌ Patch coverage is 79.26829% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.65%. Comparing base (b34af59) to head (a8532f1).

Files with missing lines Patch % Lines
fre/cmor/cmor_mixer.py 79.48% 16 Missing ⚠️
fre/cmor/cmor_helpers.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #715      +/-   ##
==========================================
- Coverage   82.76%   82.65%   -0.11%     
==========================================
  Files          68       68              
  Lines        4525     4578      +53     
==========================================
+ Hits         3745     3784      +39     
- Misses        780      794      +14     
Flag Coverage Δ
unittests 82.65% <79.26%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
fre/cmor/cmor_helpers.py 89.38% <75.00%> (ø)
fre/cmor/cmor_mixer.py 89.34% <79.48%> (-1.81%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b34af59...a8532f1. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ilaflott
Copy link
Member Author

ilaflott commented Feb 3, 2026

@ilaflott ilaflott marked this pull request as ready for review February 4, 2026 17:41
@ilaflott ilaflott changed the title CMIP7 table compatibility for fre cmor routines a CMIP7 example for fre cmor, toward general CMIP7 compatibility Feb 4, 2026
@ilaflott
Copy link
Member Author

ilaflott commented Feb 4, 2026

@copilot evaluate the compatibility of this PR with the branch in #721

Copy link
Contributor

Copilot AI commented Feb 4, 2026

@ilaflott I've opened a new pull request, #723, to work on those changes. Once the pull request is ready, I'll request review from you.

@ilaflott
Copy link
Member Author

ilaflott commented Feb 4, 2026

@copilot evaluate the compatibility of this PR with the branch in #716 by merging the two and running the CI workflow(s)

Copy link
Contributor

Copilot AI commented Feb 4, 2026

@ilaflott I've opened a new pull request, #724, to work on those changes. Once the pull request is ready, I'll request review from you.

@ilaflott ilaflott mentioned this pull request Feb 4, 2026
@ilaflott
Copy link
Member Author

ilaflott commented Feb 4, 2026

according to #724, this will not clash with #716 functionally

@ilaflott ilaflott assigned singhd789 and unassigned singhd789 Feb 5, 2026
'''
is_ci = os.environ.get("GITHUB_WORKSPACE") is not None
if is_ci:
#doesn't run happily in CI and not needed
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should there be a test failure if is_ci is True? (since you commented it doesn't run happily in CI) Or would that never really happen?

Copy link
Member Author

Choose a reason for hiding this comment

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

if is_ci throws a failure, then the pipeline fails! we wouldn't want that.

there should be added lines below this: if not is_ci: <do_stuff>. does nothing otherwise and passes without a need to explicitly assert True

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines -288 to +299
if is_ci:
#doesn't run happily in CI and not needed
assert True
else:
git_cmd = f"git restore {EXP_CONFIG}"
restore = subprocess.run(git_cmd,
shell=True,
check=False)
check_cmd = f"git status | grep {EXP_CONFIG}"
check = subprocess.run(check_cmd,
shell = True, check = False)
#first command completed, second found no file in git status
assert all([restore.returncode == 0, check.returncode == 1])
if not is_ci:
git_cmd = f"git restore {EXP_CONFIG}"
restore = subprocess.run(git_cmd,
shell=True,
check=False)
check_cmd = f"git status | grep {EXP_CONFIG}"
check = subprocess.run(check_cmd,
Copy link
Member Author

Choose a reason for hiding this comment

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

here

@ilaflott ilaflott changed the title a CMIP7 example for fre cmor, toward general CMIP7 compatibility fre.cmor: a CMIP7 example, toward general CMIP7 compatibility Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new functioning New feature or request priority: HIGH

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fre.cmor: test official CMIP7 flavored cmor tables fre.cmor: try using the few CMIP7 CMOR tables in PCMDI/cmor

3 participants