Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error checking into entry_id.py for input attributes dictionary for values that are None? #4719

Open
ekluzek opened this issue Dec 13, 2024 · 2 comments

Comments

@ekluzek
Copy link
Contributor

ekluzek commented Dec 13, 2024

We ran into this problem in CMEPS (ESCOMP/CMEPS#520) which was hard to figure out, because there wasn't error checking in place in cime, for an attribute being set to None. Adding some error checking would help users and developers identify the problem easier.

I added the following code to track down the problem. What's below is overkill, but I would cut it back to something reasonable. I think it would also be helpful if there was a DEBUG level logging on the check if attribute is not in attributes as that's likely a problem with the XML file being read in.

diff --git a/CIME/XML/entry_id.py b/CIME/XML/entry_id.py
index c090a3633..930b613d3 100644
--- a/CIME/XML/entry_id.py
+++ b/CIME/XML/entry_id.py
@@ -127,11 +127,24 @@ def _get_value_match(
                             score = -1
                             break
                     else:
-                        if attribute not in attributes or not re.search(
-                            self.get(vnode, attribute), attributes[attribute]
-                        ):
+                        if attribute not in attributes:
                             score = -1
                             break
+                        else: 
+                            expect(isinstance(attribute,str), "attribute is NOT a string:"+str(attribute))
+                            if attributes.get(attribute) == None:
+                                print( "attribute is NOT in the attributes dictionary:"+str(attribute) )
+                                print( "attributes dictionary:"+str(attributes) )
+                                print( "filename:"+str(self.filename) )
+                                print( "groups:"+str(self.groups) )
+                            else:
+                                expect(isinstance(attributes[attribute],str), "attribute from the attributes dictionary does NOT return a string:"+str(attribute))
+
+                            if not re.search(
+                               self.get(vnode, attribute), attributes[attribute]
+                            ):
+                               score = -1
+                               break
 
             # Add valid matches to the list.
             if score >= 0:
@ekluzek
Copy link
Contributor Author

ekluzek commented Dec 13, 2024

@jedwards4b @fischer-ncar @billsacks @jgfouca @rljacob and whoever else -- do you have thoughts on this? If there's consensus that this would be good, I could make a small PR to bring this in.

@jedwards4b
Copy link
Contributor

@ekluzek I don't believe that this is the correct fix. The problem is not in setting a variable to None - I believe that this should be an acceptable setting. The issue is that the namelist xml code does not allow a variable to be set to None - if there is to be a change in cime I believe that it should be there and not here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants