Skip to content

Drop trailing commas that made five argsDict values 1-tuples - #44

Merged
cekees merged 1 commit into
mainfrom
fix-argsdict-tuples
Oct 13, 2026
Merged

cekees merged 1 commit into
mainfrom
fix-argsdict-tuples

Conversation

@cekees

@cekees cekees commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

argsDict["k"] = arr, stores the 1-tuple (arr,). On main, proteus::pyarray rejects tuples, so each of these raised TypeError: __setitem__(): incompatible function arguments the first time its method ran:

site method key
proteus/m_comp_co2/m_comp_co2.py:3830 LevelModel.getResidual elementMaterialTypes
proteus/m_comp_co2/m_comp_co2.py:4677 LevelModel.getJacobian elementMaterialTypes
proteus/mprans/RANS2P_IB.py:1343 LevelModel.getJacobian elementDiameter
proteus/mprans/RANS2P_IB.py:1574 LevelModel.beamStep elementDiameter
proteus/mprans/SW2D.py:825 LevelModel.getJacobian elementDiameter

None of these is behind a condition, so every run of m_comp_co2, RANS2P_IB or SW2D failed on its first residual or Jacobian evaluation. They went unnoticed because nothing in test/ or proteus/tests/ runs any of the three models.

27c00b0 fixed the other eleven occurrences listed in the report (PresInc, RANS2P_IB sdInfo_u_w_rowptr, RANS3PF, RANS3PSed, richards/ADR, Richards). It missed these five for two reasons: the RANS2P_IB and SW2D lines have a # comment after the comma, which a line-end grep misses, and m_comp_co2 was merged the next day (82ce489).

Guard

test/test_argsdict_values.py parses every .py under the installed proteus package with ast. It fails on any tuple, list, set, dict, comprehension, None or string assigned into argsDict[...] or into any name bound to ArgumentsDict(). The check doesn't depend on a model having a test, and it handles comments and multi-line values. A second test feeds the scanner a synthetic snippet, so the scanner can't silently stop matching.

Verified locally: both tests pass on this branch. The scan fails on unfixed origin/main (exactly these 5 sites) and on unfixed origin/1.9.x (12 sites plus ADR.py).

Companion 1.9.x PR: #45.

🤖 Generated with Claude Code

`argsDict["k"] = arr,` stores (arr,). proteus::pyarray rejects a tuple, so
each of these raised TypeError from ArgumentsDict.__setitem__ the first
time its method ran:

- m_comp_co2.LevelModel.getResidual / getJacobian: elementMaterialTypes
- RANS2P_IB.LevelModel.getJacobian / beamStep: elementDiameter
- SW2D.LevelModel.getJacobian: elementDiameter

All five are unconditional, so every run of these models failed on its
first residual or Jacobian. Nothing in test/ runs m_comp_co2, RANS2P_IB or
SW2D. 27c00b0 fixed the other eleven; the RANS2P_IB and SW2D lines
have a comment after the comma, which a line-end grep for "," misses,
and m_comp_co2 was merged the day after (82ce489).

test/test_argsdict_values.py walks the installed proteus sources with ast
and rejects any tuple, list, set, dict, comprehension, None or string
assigned into an ArgumentsDict, so no future occurrence depends on a model
having a test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cekees
cekees merged commit 951581e into main Oct 13, 2026
5 checks passed
@cekees
cekees deleted the fix-argsdict-tuples branch October 13, 2026 14:26
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.

1 participant