Fix handling of namelist variables with uppercase characters #4881
+169
−21
Conversation
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
Prior to this commit, variables with uppercase characters in the namelist definition file could not be modified via user_nl files (regardless of whether the entry in the user_nl file matched the correct case or was all lowercase). This commit fixes this behavior so that entries in the user_nl file are case-insensitive, while preserving the case as defined in the namelist definition file in the final output.
Extract a function that will be helpful for other unit tests
I realized that there were two problems with my recent implementation of case insensitivity in dict_to_namelist: - If a variable didn't exist in the namelist definition, a cryptic error would be generated from the attempt to set `variable_actual_case = self._entry_ids_lower_to_actual[variable_name.lower()]` before getting a chance to generate the more user-friendly error message via `_expect_variable_in_definition`. - If a variable had an array slice (e.g., `foo(3) = 4`), the earlier code wouldn't work right, because this full variable name with the slice wouldn't exist in `_entry_ids_lower_to_actual`. This commit solves both of those problems by determining the qualified varname before indexing into `_entry_ids_lower_to_actual`. This requires a bit of extra complexity to then form a version of the original variable name (with any array slice included) in the appropriate case; this is done via a string `replace` call.
Add unit tests covering various scenarios of having mixed-case variables in the namelist definition (which was fixed in recent commits).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4881 +/- ##
==========================================
+ Coverage 30.98% 31.02% +0.03%
==========================================
Files 264 264
Lines 38663 38677 +14
Branches 8384 8384
==========================================
+ Hits 11979 11998 +19
+ Misses 25459 25455 -4
+ Partials 1225 1224 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
billsacks
commented
Oct 18, 2025
| # actually be needed, but I'm setting it to maintain some old logic, to be safe. | ||
| value = super(NamelistDefinition, self).get_value_match( | ||
| vid.lower(), | ||
| vid, |
Member
Author
There was a problem hiding this comment.
The removal of .lower() here and in the call from get_default_value aren't needed to get things to work, but I think could be needed in principle in the future: I think these weren't needed because (1) get_default_value doesn't seem to ever be called, and (2) it happens that the vid here is irrelevant because we always pass an entry_node into this call.
I tested removing the setting of entry_node a couple of lines above. In this case, I got an error in get_value_match unless I removed the lower here, when working with mixed-case variable names.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Prior to this PR, variables with uppercase characters in the namelist definition file could not be modified via user_nl files (regardless of whether the entry in the user_nl file matched the correct case or was all lowercase).
This PR fixes this behavior so that entries in the user_nl file are case-insensitive, while preserving the case as defined in the namelist definition file in the final output. (Note that the case of variable names in any explicit
add_defaultcalls will still need to match the case of the variable in the namelist definition file.)Checklist
Testing performed
(1) scripts_regression_tests on my Mac
Failures were the same as on main
(2) create_test with baseline comparisons, namelist generation/comparison only (
-nflag) for these tests in a CESM context:(3) Manual testing of setting a mixed-case variable in the
user_nlfile: setlogFilePostFixinuser_nl_cpl.Set this as
logFilePostFix,logfilepostfixandlogfilepostFIX. Confirmed that all worked, and all led to the final output innuopc.runconfigmatching the case in the namelist definition file (i.e.,logFilePostFix).