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

Check that weight variables exist #133

Merged

Conversation

phackstock
Copy link
Contributor

Closes #132.

Features added

The validation of the variable code list now includes a check that all variables mentioned as a weight also have their own entry in the list. I've written the validator so that it collects all missing weight variables and does not throw an error for the first one. This way multiple errors can be fixed in one go.
I added a point in the documentation to highlight that the weight is required.
I also took the liberty to slightly refactor the codelist validators. The main change there is the switch from using root_validator() to validator("mapping").
The reason for this change is that in the validators only modify and validate the mapping attribute. The name attribute is mainly used for better error reporting in this case. The way how to access multiple attributes in a non-root validator is adding v and values to the list of input variables (details can be found under the code example of "Validators" https://pydantic-docs.helpmanual.io/usage/validators/). As an example:

@validator("mapping")
def some_validator(cls, v, values):
    # v is the field to be validated "mapping" in our case
    # values is a dict, that contains all other previously validated fields

If you don't like the switch from the root_validator though I can also revert it.
Looking forward to your comments.

@phackstock phackstock self-assigned this Jul 5, 2022
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.

A nice improvement, thanks! Just a few suggestions for shorter, more readable documentation and error messages.

doc/source/user_guide/codelist.rst Outdated Show resolved Hide resolved
nomenclature/error/variable.py Outdated Show resolved Hide resolved
tests/test_definition_variable.py Outdated Show resolved Hide resolved
@phackstock phackstock merged commit 4dc3a7c into IAMconsortium:main Jul 8, 2022
@phackstock phackstock deleted the feature/variable-weight-check branch July 8, 2022 11:21
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.

Check that weight variables exist
2 participants