-
Notifications
You must be signed in to change notification settings - Fork 14
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
Extend stray tag check #411
Extend stray tag check #411
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, see a suggestion for cleaner test-code inline.
Also, I suggest to also add a test-case where a stray tag is in a attribute-string, like
- Primary Energy:
definition: Total primary energy { consumption
unit: EJ/yr
Initially I had added one, but removed it since the original test does so by checking for the stray tag in the code name. Can easliy add another more explicit one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more little suggestionfor better error messages, then good to be merged.
nomenclature/codelist.py
Outdated
if isinstance(value, str): | ||
if any(char in value for char in forbidden): | ||
raise ValueError( | ||
f"Unexpected bracket in codelist: {code.name}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f"Unexpected bracket in codelist: {code.name}." | |
f"Unexpected bracket in {self.name} '{code.name}'." |
tests/test_codelist.py
Outdated
@pytest.mark.parametrize( | ||
"subfolder, match", | ||
[ | ||
("char_in_str", r"Unexpected bracket in codelist: Primary Energy\|{Feul}"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
("char_in_str", r"Unexpected bracket in codelist: Primary Energy\|{Feul}"), | |
("char_in_str", r"Unexpected bracket in variable 'Primary Energy\|{Feul}'"), | |
tests/test_codelist.py
Outdated
"subfolder, match", | ||
[ | ||
("char_in_str", r"Unexpected bracket in codelist: Primary Energy\|{Feul}"), | ||
("char_in_list", r"Unexpected bracket in codelist: Share\|Coal"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
("char_in_list", r"Unexpected bracket in codelist: Share\|Coal"), | |
("char_in_list", r"Unexpected bracket in variable 'Share\|Coal'"), |
tests/test_codelist.py
Outdated
[ | ||
("char_in_str", r"Unexpected bracket in codelist: Primary Energy\|{Feul}"), | ||
("char_in_list", r"Unexpected bracket in codelist: Share\|Coal"), | ||
("char_in_dict", r"Unexpected bracket in codelist: Primary Energy"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
("char_in_dict", r"Unexpected bracket in codelist: Primary Energy"), | |
("char_in_dict", r"Unexpected bracket in variable 'Primary Energy'"), |
Partial resolution of #401.
Expands on stray tag check in code names, adding checks for brackets for all code attributes.
Recursively checks list and dict attributes for strings.
Tests expanded.