Skip to content

delete redundant height_0 in variables coordinates attrs#410

Merged
rbeucher merged 2 commits into
mainfrom
fix_height_0_residual_issue
Jun 2, 2026
Merged

delete redundant height_0 in variables coordinates attrs#410
rbeucher merged 2 commits into
mainfrom
fix_height_0_residual_issue

Conversation

@rhaegar325
Copy link
Copy Markdown
Collaborator

Issue to be fixed

The CMORised output for tas, hurs, huss (Amon) has

tas:coordinates = "height_0 height" ;

but only height is actually written to the file. height_0 is a dangling
reference, flagged by WCRP compliance-checker:

[ATTR004] Variable 'tas' attribute 'coordinates' as-variable check
  Referenced variables not found: ['height_0']

Fix

Replace the merge with a token-aware, dataset-validated one in
src/access_moppy/base.py:

if var == self.cmor_name:
    existing_tokens = vdat.attrs.get("coordinates", "").split()
    valid_existing = [t for t in existing_tokens if t in self.ds.variables]
    merged = list(dict.fromkeys(valid_existing + list(aux_coords)))
    if merged:
        v.setncattr("coordinates", " ".join(merged))
    elif "coordinates" in v.ncattrs():
        v.delncattr("coordinates")
  • Drops tokens not present in self.ds.variables (kills the dangling
    height_0).
  • Deduplicates with dict.fromkeys (preserves order).
  • Removes the attribute entirely when nothing valid remains, instead of
    leaving a dangling reference inherited from line 1047's attribute copy.

Scope of the Change

Single block in src/access_moppy/base.py. Behaviour identical to the
previous code in every scenario where the previous output was already correct;
strictly improves the cases that previously produced dangling or duplicate
tokens. No existing tests assert on the output coordinates attribute, so no
test changes are required (a regression test is recommended but not blocking).

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.2%. Comparing base (34d58b9) to head (2539737).

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #410     +/-   ##
=======================================
+ Coverage   75.1%   75.2%   +0.1%     
=======================================
  Files         28      28             
  Lines       5281    5282      +1     
  Branches     973     973             
=======================================
+ Hits        3966    3972      +6     
+ Misses      1091    1089      -2     
+ Partials     224     221      -3     
Flag Coverage Δ
unit 75.2% <100.0%> (+0.1%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@rhaegar325 rhaegar325 requested a review from rbeucher May 29, 2026 05:23
@rbeucher rbeucher merged commit ad2e247 into main Jun 2, 2026
4 checks passed
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.

2 participants