Skip to content

fix(mflistfile): raise informative error if first budget can't be parsed - #2859

Open
wpbonelli wants to merge 3 commits into
modflowpy:developfrom
wpbonelli:fix-listbudget-first-entry-error
Open

wpbonelli wants to merge 3 commits into
modflowpy:developfrom
wpbonelli:fix-listbudget-first-entry-error

Conversation

@wpbonelli

@wpbonelli wpbonelli commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

If ListBudget failed to parse a value in the first budget table, rather than raise an error, it would just print a message and continue with bad internal state, eventually raising a misleading unpacking error wrapped in a generic Exception. The only mention of the actual problem was the printed line, which easily gets buried under the unhelpful exceptions.

If any values in the first budget table fail to parse, raise ValueError naming the time step, stress period, and invalid line. I left behavior for parse failures in later budget tables unchanged: print a message and fill that time step with NaN. This means the same bad value will raise in the first table but become NaN in any other. This inconsistency isn't good (neither is giving NaN for bad values, IMO, since it makes it impossible to distinguish legitimate NaNs from parse failures), but people may have come to expect and rely on this behavior, so it seems safest to leave a comprehensive fix for 4.x.

Also reword list file parsing messages: "time step x, stress period y" instead of "ts,sp x y", readable names (e.g. "time step length" instead of "tslen"), and report the bad line where available.

This situation will now raise an error like

ValueError: unable to read first budget in list file model.lst: could not parse value for FLOW-JA-FACE (OUT) at time step 1, stress period 1: 'FLOW-JA-FACE =       8.5159-100 ...'

@wpbonelli wpbonelli added this to the 3.11.1 milestone Sep 28, 2026
@wpbonelli wpbonelli added the bug label Sep 28, 2026
@wpbonelli
wpbonelli marked this pull request as ready for review September 28, 2026 20:21
@codecov

codecov Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.69231% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.8%. Comparing base (556c088) to head (b3c55a2).
⚠️ Report is 236 commits behind head on develop.

Files with missing lines Patch % Lines
flopy/utils/mflistfile.py 57.6% 11 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2859      +/-   ##
===========================================
+ Coverage     55.5%    73.8%   +18.2%     
===========================================
  Files          644      659      +15     
  Lines       124135   132404    +8269     
===========================================
+ Hits         68947    97732   +28785     
+ Misses       55188    34672   -20516     
Files with missing lines Coverage Δ
flopy/utils/mflistfile.py 73.9% <57.6%> (+4.4%) ⬆️

... and 585 files with indirect coverage changes

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

wpbonelli and others added 3 commits September 28, 2026 18:36
ListBudget reads budget entry names from the first budget table. If a
value in it could not be parsed, _get_sp printed a message and returned
the (still empty) null entries, causing an unrelated unpacking error
that was then wrapped in a generic Exception. Now a ValueError is raised
naming the time step, stress period, and offending line. Behavior for
later budget tables (print and fill with NaN) is unchanged.

Close modflowpy#2856

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Name the budget entry by its column key (e.g. FLOW-JA-FACE_OUT), spell
out time step and stress period, and use one message for unparseable
rate or cumulative values since the offending line is included.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Label budget entries by name and section, e.g. FLOW-JA-FACE (OUT), with
summary lines (TOTAL IN, PERCENT DISCREPANCY) unlabeled. Use the same
"time step x, stress period y" wording in time summary and index
messages, and report the actual line number when a budget header's
time step and stress period can't be parsed (previously always 1).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wpbonelli
wpbonelli force-pushed the fix-listbudget-first-entry-error branch from 4255ecb to b3c55a2 Compare September 29, 2026 01:43

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant