Conversation
Codecov Report❌ Patch coverage is
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
🚀 New features to boost your workflow:
|
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
force-pushed
the
fix-listbudget-first-entry-error
branch
from
September 29, 2026 01:43
4255ecb to
b3c55a2
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If
ListBudgetfailed 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 genericException. 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
ValueErrornaming 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 becomeNaNin any other. This inconsistency isn't good (neither is givingNaNfor bad values, IMO, since it makes it impossible to distinguish legitimateNaNs 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