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

Replace tags in attribute lists and dicts #352

Conversation

korsbakken
Copy link
Collaborator

This PR (hopefully) solves issue #342 . It modifies Code.replace_tag to descend into attribute lists and dicts, replaing each string in list elements of dict values that contain a tag. Adds a test and test data that contains a code with tags in the components attribute.

Also corrected the test from the previous commits. The `components` attribute
is a list of one-element dictionaries in the yaml file, but is apparently
flattened during parsing and becomes a simple dict instead.
Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @korsbakken, very nice - just a few suggestions for consistency...

tests/data/tagged_codelist/foo_attr_list.yaml Outdated Show resolved Hide resolved
tests/data/tagged_codelist/tag_species.yaml Outdated Show resolved Hide resolved
tests/data/tagged_codelist/tag_species.yaml Outdated Show resolved Hide resolved
tests/test_codelist.py Outdated Show resolved Hide resolved
tests/test_codelist.py Outdated Show resolved Hide resolved
tests/test_codelist.py Outdated Show resolved Hide resolved
nomenclature/code.py Outdated Show resolved Hide resolved
nomenclature/code.py Outdated Show resolved Hide resolved
@korsbakken
Copy link
Collaborator Author

Thanks @korsbakken, very nice - just a few suggestions for consistency...

Thanks @danielhuppmann, those suggestions make good sense, I have added and committed them to the PR now.

@korsbakken
Copy link
Collaborator Author

The suggested changes appear to cause tests to fail, not clear why. It introduces an unintentional blank line in the middle of a definition, and an inconsistency in the CO2 and CH4 unit definitions, but new less obvious failures appear after fixing that.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, the "/yr" was specified both in the code itself and the tag...

tests/data/tagged_codelist/foo_attr_list.yaml Show resolved Hide resolved
tests/data/tagged_codelist/tag_species.yaml Outdated Show resolved Hide resolved
The last commit contained an unintentional blank line in test_codelist.py, and
inconsistent indentation in data/tagged_codelis/tag_species.yaml that was
causing an error when creating the CodeList.
Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@danielhuppmann danielhuppmann merged commit bccd352 into IAMconsortium:main Jul 3, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants