CFReader._build_cf_groups: "is CFBoundaryVariable" identity test can never be True
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 70/100
Research direction
The bug is in lib/iris/fileformats/cf.py lines 1662-1665. First, understand the CFGroup class and its getitem method (around line 1241). The fix likely changes 'is' to 'isinstance', but check the intent from PR #6481. Run existing tests to ensure no regression, and add a test under iris.FUTURE.derived_bounds to verify the corrected behavior.
Written by the indexing model from the issue text.
Description
🐛 Bug Report
How to reproduce
Not reproducible as a user-visible symptom — the report is that a guard added
under iris.FUTURE.derived_bounds is dead code and has never run.
In CFReader._build_cf_groups
(lib/iris/fileformats/cf.py:1662-1665):
for cf_var in self.cf_group.formula_terms.values():
if iris.FUTURE.derived_bounds:
if self.cf_group[cf_var.cf_name] is CFBoundaryVariable:
continue
CFGroup.__getitem__ returns a CFVariable instance — CFGroup.__setitem__
enforces isinstance(variable, CFVariable) and raises TypeError otherwise
(cf.py:1241-1246). The is comparison tests that instance against the
CFBoundaryVariable class, so it is always False and the continue has
never executed.
What actually happens
Under iris.FUTURE.derived_bounds, a formula-terms variable that is also
classified as a bounds variable is still considered for promotion to a
CFDataVariable.
What I expected to happen
Whatever the guard was written to do — presumably skip those variables. The
intended test looks like isinstance(self.cf_group[cf_var.cf_name], CFBoundaryVariable),
but the right fix depends on the intent rather than on the mechanics, hence
this issue rather than a pull request.
Provenance
Introduced by #6481 (Pp derived bounds, 2025-06-02). Because the branch has
never been taken, no existing test distinguishes the two behaviours, and
changing is to isinstance would be a behaviour change under
iris.FUTURE.derived_bounds that needs its own tests and a changelog fragment.
Found while planning the split of iris.fileformats.cf into a package
(part of #6977). That pull request is a behaviour-preserving move, so it
carries a comment pointing at this issue rather than a fix.
cc @pp-mo
- Dominant language
- Python
- Stars
- 725
- Forks
- 317
- Avg merge
- 3d 4h
- Merged PRs (30d)
- 26
Getting set up
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from SciTools/iris
-
Difficulty 2/5 1-3 hours Newbie friendliness 90/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
Maintainers usually reply within 1 day
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
kornia/kornia#5263 · 1 comment ·
Maintainers usually reply within 1 day
-
approved correction metadata
Difficulty 1/5 Under an hour Newbie friendliness 88/100
acl-org/acl-anthology#10133 · 1 comment ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
BasedHardware/omi#20084 ·
Maintainers usually reply within 1 day
-
bug needs-acceptance wg/evaluation-quality
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
vllm-project/semantic-router#4424 ·
Maintainers usually reply within 1 day