Skip to content

fre.cmor: variable list semantics: map modeler variable names to MIP table names#846

Open
Copilot wants to merge 14 commits intomainfrom
copilot/update-variable-list-usage
Open

fre.cmor: variable list semantics: map modeler variable names to MIP table names#846
Copilot wants to merge 14 commits intomainfrom
copilot/update-variable-list-usage

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 8, 2026

  • Create CDL file for mapped-variable test (sea_sfc_salinitysos)
  • Create varlist JSON for mapped-variable test
  • Add conftest.py to fre/cmor/tests/ with shared fixtures (paths, ncgen setup, temp dirs)
  • Refactor test_cmor_run_subtool.py to use fixtures and tmp_path
  • Add case3 mapped-variable tests in test_cmor_run_subtool.py (success + data/metadata comparison)
  • Fix CLI error propagation in frecmor.py (check return status, raise ClickException)
  • Add conftest.py to fre/tests/ with shared CLI fixtures
  • Refactor test_fre_cmor_cli.py to use conftest fixtures and tmp_path
  • Add case3 mapped-variable tests in test_fre_cmor_cli.py (CMIP6 + CMIP7)
  • Run all tests: 116 cmor tests pass, 144 fre/tests pass (1 skip, 2 xfail)
  • Run validation and address review comments

Copilot AI and others added 4 commits April 8, 2026 19:02
… names

- Change varlist semantics: key = modeler's var name (in filename AND file),
  value = MIP table variable name
- Add validation: error when filename variable != file variable
- Read variable data from file using local_var (modeler name) instead of target_var
- Update CLI run command to propagate non-zero return status as ClickException
- Update test case2 to test error case (filename/file variable mismatch)
- Update docstrings, help text, and log messages to reflect new semantics

Agent-Logs-Url: https://github.com/NOAA-GFDL/fre-cli/sessions/35f4cd66-b4ff-4394-aa91-d318347d10f1

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Copilot AI changed the title [WIP] Adjust current variable list usage to map modeler variable names to MIP Adjust fre.cmor variable list semantics: map modeler variable names to MIP table names Apr 8, 2026
Copilot AI requested a review from ilaflott April 8, 2026 19:05
@ilaflott
Copy link
Copy Markdown
Member

ilaflott commented Apr 8, 2026

@copilot tests are running but we've plenty to do- while you wait for results, find content under docs/ regarding fre.cmor and it's variable list behavior, update the text to match the target behavior we are developing for. if the tests finish unsuccessfully during your iteration, don't stop re-evaluating the docs. we will have another iteration for addressing any test failures if-needed

Copilot AI and others added 2 commits April 8, 2026 19:35
…ble var

Update documentation across docs/usage/cmor_cookbook.rst, docs/tools/cmor.rst,
and fre/cmor/README.md to reflect the new variable list semantics where:
- key = modeler's variable name (in filename AND inside the file)
- value = MIP table variable name
- filename var must match file var or fre cmor run errors

Agent-Logs-Url: https://github.com/NOAA-GFDL/fre-cli/sessions/5870f988-b97c-4d7b-8b29-e36e63dfb14c

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 8, 2026

@copilot tests are running but we've plenty to do- while you wait for results, find content under docs/ regarding fre.cmor and it's variable list behavior, update the text to match...

Updated all documentation under docs/ and fre/cmor/README.md to reflect the new variable list behavior in commit 616d595:

  • docs/usage/cmor_cookbook.rst: Rewrote "Creating Variable Lists" section with clear key/value semantics (key=modeler var, value=MIP table var), added .. important:: note about filename/file variable matching requirement
  • docs/tools/cmor.rst: Updated --varlist description from "local to MIP" to "modeler variable names to MIP table variable names"
  • fre/cmor/README.md: Updated Required User Configuration section with full key/value explanation, updated fre cmor run description to clarify --varlist usage, updated fre cmor varlist section to describe the default identity mapping and editing workflow

@ilaflott ilaflott changed the title Adjust fre.cmor variable list semantics: map modeler variable names to MIP table names fre.cmor: variable list semantics: map modeler variable names to MIP table names Apr 8, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.17%. Comparing base (836559e) to head (bed2b1f).

Files with missing lines Patch % Lines
fre/cmor/cmor_finder.py 0.00% 2 Missing ⚠️
fre/cmor/cmor_mixer.py 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #846      +/-   ##
==========================================
+ Coverage   84.12%   84.17%   +0.05%     
==========================================
  Files          71       71              
  Lines        4944     4949       +5     
==========================================
+ Hits         4159     4166       +7     
+ Misses        785      783       -2     
Flag Coverage Δ
unittests 84.17% <81.25%> (+0.05%) ⬆️

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

Files with missing lines Coverage Δ
fre/cmor/frecmor.py 100.00% <100.00%> (ø)
fre/cmor/cmor_mixer.py 90.66% <90.90%> (+0.79%) ⬆️
fre/cmor/cmor_finder.py 97.80% <0.00%> (-2.20%) ⬇️

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 836559e...bed2b1f. Read the comment docs.

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

Copilot AI and others added 3 commits April 13, 2026 17:13
…h fixtures

- Create CDL file and varlist for mapped variable test (sea_sfc_salinity → sos)
- Add conftest.py with shared session-scoped fixtures for ncgen, paths, file setup
- Refactor test_cmor_run_subtool.py to use conftest fixtures and tmp_path
- Add case3 tests: success, data comparison, and metadata comparison
- Fix CLI error propagation in frecmor.py (check cmor_run_subtool return status)

Agent-Logs-Url: https://github.com/NOAA-GFDL/fre-cli/sessions/80cfd2fa-1f68-407c-b0d7-cde17e9e4b1b

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
…e tests

- Add conftest.py for fre/tests/ with shared CLI fixtures
- Refactor test_fre_cmor_cli.py to use conftest fixtures and tmp_path
- Add case3 mapped-variable tests for both CMIP6 and CMIP7
- All 35 CLI tests pass, all 116 cmor tests pass, all 144 fre/tests pass

Agent-Logs-Url: https://github.com/NOAA-GFDL/fre-cli/sessions/80cfd2fa-1f68-407c-b0d7-cde17e9e4b1b

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
@ilaflott ilaflott marked this pull request as ready for review April 13, 2026 18:55
@ilaflott
Copy link
Copy Markdown
Member

this is about right.

i want to adjust fre cmor varlist too for the new approach, but this PR is already bloated, so i'm cutting myself off.

@ilaflott
Copy link
Copy Markdown
Member

fremorizer is still poking at this, keep for now

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: adjust current variable list usage to map modeler variable names to MIP ones

2 participants