Fix TypeError when reporting ParseSyntaxError in parse_fortran_var_decl - #726
Merged
Conversation
nusbaume
reviewed
Mar 15, 2026
nusbaume
left a comment
Collaborator
There was a problem hiding this comment.
The source code change looks good to me, but should we add some sort of doctest or unit test to ensure that the error you ran into will continue to be caught correctly in the future?
Contributor
Author
|
Thanks @nusbaume for the suggestion, I added a doctest that should cover this change If the change is not applied, this new test would fail: |
nusbaume
approved these changes
Mar 16, 2026
nusbaume
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the new test @jimmielin! Everything looks good to me now (albeit as a non-framework expert).
climbfuji
approved these changes
Mar 23, 2026
mkavulich
approved these changes
Mar 23, 2026
mkavulich
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the fix and test!
climbfuji
pushed a commit
to climbfuji/ccpp-framework
that referenced
this pull request
Apr 3, 2026
…cl (NCAR#726) Fix TypeError from ParseSyntaxError in parse_fortran_var_decl In `parse_fortran_var_decl()`, when `FortranVar()` raises a `ParseSyntaxError`, the exception object was appended directly to the errors list. `parse_module` expects string elements and calls `'\n'.join(errors)`, which crashes with a `TypeError`. Convert the exception to a string before appending. From the context above `errmsg` is already being appended to `errors` array so I think this is a safe change to make that is consistent with the intended type of `errors[]`. User interface changes?: No Fixes: closes NCAR#725 Testing: unit tests: New doctest for parse_fortran_var_decl() covers previous error manual testing: see minimal reproducible example in issue NCAR#725
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.
[ 50 character, one line summary ]
Fix TypeError from ParseSyntaxError in parse_fortran_var_decl
[ Description of the changes in this commit. It should be enough
information for someone not following this development to understand.
Lines should be wrapped at about 72 characters. ]
In
parse_fortran_var_decl(), whenFortranVar()raises aParseSyntaxError, the exception object was appended directlyto the errors list.
parse_moduleexpects string elements and calls'\n'.join(errors), which crashes with aTypeError. Convertthe exception to a string before appending.
From the context above
errmsgis already being appended toerrorsarray so I think this is a safe change to make that isconsistent with the intended type of
errors[].User interface changes?: No
Fixes: closes #725
Testing:
test removed:
unit tests:
system tests:
manual testing: see minimal reproducible example in issue #725