Do not add empty user mods #1675
Conversation
pylint wants this
pylint 1.6.4 on yellowstone kept giving me errors like this: E:1125,15: Instance of 'int' has no 'isspace' member (no-member) E:1125,15: Instance of 'float' has no 'isspace' member (no-member) I tried lots of things, and this EAFP approach is the one way I can make pylint happy
|
cc @mvertens |
scripts/lib/CIME/case.py
Outdated
| is_space = comp_user_mods.isspace() | ||
| except AttributeError: | ||
| is_space = False | ||
| if is_space: |
There was a problem hiding this comment.
Checking is_space isn't necessary right now, but is necessary to avoid cryptic error messages if someone had something like <default_value> </default_value> (note the space)
scripts/lib/CIME/case.py
Outdated
| if comp_user_mods is None or comp_user_mods == "": | ||
| return None | ||
| else: | ||
| try: |
There was a problem hiding this comment.
After much trial and error, this try/except block was the only way I could get pylint 1.6.4 (on yellowstone) to accept the use of isspace. Without it, pylint gave errors like this:
E:1125,15: Instance of 'int' has no 'isspace' member (no-member)
E:1125,15: Instance of 'float' has no 'isspace' member (no-member)
scripts/lib/CIME/case.py
Outdated
| Returns None if no value was found, or if the value is an empty string. | ||
| """ | ||
| comp_user_mods = self.get_value("{}_USER_MODS".format(component.upper())) | ||
| if comp_user_mods is None or comp_user_mods == "": |
There was a problem hiding this comment.
Ideally it seems like we'd have some shared handling of empty strings in some common place, but I'm not sure where this should go (e.g., one of the get_value routines somewhere in the inheritance chain?), and I didn't want to risk breaking things this late in the release cycle. So for now I'm just applying this special handling for this one case where I know we need it.
| Returns None if no value was found, or if the value is an empty string. | ||
| """ | ||
| comp_user_mods = self.get_value("{}_USER_MODS".format(component.upper())) | ||
| #pylint: disable=no-member |
There was a problem hiding this comment.
I think that this is cleaner than the exception code - do you agree?
There was a problem hiding this comment.
This is definitely cleaner. But I was hesitant to bypass this pylint check, figuring that pylint might be seeing some real possibility for problems. In particular: If there is any possibility that comp_user_mods is not a string here, then this code will raise an exception. I'll defer to your understanding here: If you're confident that get_value will always return a string here, then I'm happy with your change.
There was a problem hiding this comment.
It should always return a string and if it doesn't it should be an exception.
Ignore empty COMP_USER_MODS values
If a component defined a COMPNAME_USER_MODS (e.g., CAM_USER_MODS) entry
in config_component.xml, but the value for the current case ends up
being blank, we were getting incorrect behavior: The expected behavior
is that no user mods directory would be included; the actual behavior
was that it tries to pull in user mods from
PRIMARY_COMPONENT/cime_config/usermods_dirs . This often will be
harmless, because that directory won't contain any user_nl files or
others (it typically just contains some subdirectories with the actual
user mods). But it's confusing, and it could give wrong behavior if that
directory happened to include any files that matched names that are
expected in a user_mods directory (e.g., user_nl files).
This PR fixes this behavior, so that if COMPNAME_USER_MODS evaluates to
an empty string, then we don't try to include it.
Test suite: scripts_regression_tests on yellowstone
Also, manual tests of create_newcase:
Test baseline: N/A
Test namelist changes: none
Test status: bit for bit
Fixes #1673
User interface changes?: none
Code review: