Skip to content

Harden branded-variable disambiguations#86

Open
Copilot wants to merge 4 commits intomainfrom
copilot/improve-branded-variable-handling
Open

Harden branded-variable disambiguations#86
Copilot wants to merge 4 commits intomainfrom
copilot/improve-branded-variable-handling

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 17, 2026

Describe your changes

Port of noaa-gfdl/fre-cli#832. Brand extraction used split('_') indexing, which breaks when target_var contains underscores and raises IndexError on non-branded MIP entries. Error messages on disambiguation failure were either empty (bare ValueError) or lacked diagnostic context.

Brand extraction (cmor_mixer.py):

  • Replace mip_var.split('_')[0] / [1] with startswith(target_var + '_') + prefix stripping
  • Guard against empty brand suffix (e.g. trailing _)
  • Bare ValueError → message with target var, dim count, available branded entries
# Before — breaks on target_var="sea_surface_temp"
target_var == mip_var.split('_')[0]        # "sea" != "sea_surface_temp"
brands.append(mip_var.split('_')[1])       # IndexError on "sos"

# After
brand_prefix = target_var + '_'
if mip_var.startswith(brand_prefix):
    brand = mip_var[len(brand_prefix):]

Disambiguation errors (cmor_helpers.py):

  • Add empty-brands-list guard at filter_brands() entry
  • "All eliminated" and "multiple remain" errors now include has_time_bnds, input_vert_dim, candidate brands, and actionable guidance

Tests (test_cmor_helpers.py):

  • test_filter_brands_empty_brands_list
  • test_filter_brands_all_eliminated_message_contains_properties
  • test_filter_brands_multiple_remain_message_contains_properties

Issue ticket number and link

Checklist before requesting a review

  • I ran this code and know what it does
  • I commented the code and think it is readable
  • I wrote new tests to cover newly added lines
  • I wrote new documentation to explain new functionality
  • pylint and pytest checks are passing
  • No print statements; all user-facing info uses logging module

Note: If you are a maintainer updating the tag or releasing a new version, use the release_procedure.md template.
To quickly use this template, open a new pull request, choose your branch, and add ?template=release_procedure.md
to the end of the url.

Copilot AI and others added 2 commits April 17, 2026 20:24
…e error messages, add guards

- cmor_mixer.py: Replace split('_') brand extraction with startswith + prefix
  stripping to handle target vars containing underscores; guard empty brand
  suffix; bare ValueError → informative message with target var, dim count,
  available branded entries
- cmor_helpers.py: Add empty-brands-list guard at filter_brands() entry;
  include has_time_bnds, input_vert_dim, candidate brands in error messages
- test_cmor_helpers.py: Update match strings for new error messages; add tests
  for empty brands list, message properties in all-eliminated and
  multiple-remain error paths

Agent-Logs-Url: https://github.com/ilaflott/fremorizer/sessions/bf9614df-0bbd-4e9b-a033-67e69d65550b

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Copilot AI changed the title [WIP] Enhance handling of branded variable name disambiguation Harden branded-variable disambiguations Apr 17, 2026
Copilot AI requested a review from ilaflott April 17, 2026 20:27
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 17, 2026

Codecov Report

❌ Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
fremorizer/cmor_mixer.py 78.57% 3 Missing ⚠️
@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
- Coverage   95.19%   95.14%   -0.05%     
==========================================
  Files          10       10              
  Lines        1310     1318       +8     
==========================================
+ Hits         1247     1254       +7     
- Misses         63       64       +1     
Files with missing lines Coverage Δ
fremorizer/cmor_helpers.py 94.28% <100.00%> (+0.03%) ⬆️
fremorizer/cmor_mixer.py 91.93% <78.57%> (-0.10%) ⬇️

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 ef228ea...c27038f. Read the comment docs.

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

@ilaflott ilaflott deleted the branch main April 20, 2026 18:30
@ilaflott ilaflott closed this Apr 20, 2026
@ilaflott ilaflott reopened this Apr 20, 2026
@ilaflott ilaflott changed the base branch from more_human_review_and_tweaks to main April 20, 2026 18:34
@ilaflott ilaflott marked this pull request as ready for review April 20, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new features: Harden branded-variable disambiguations

3 participants