-
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
Update RequiredDataValidator #281
Update RequiredDataValidator #281
Conversation
ac55744
to
bb82a48
Compare
I think it's confusing to have an error, then a warning - better to combine this into one log item. Also, you could use the (new) pyam utility |
Maybe also put the required-file-path somewhere else to make it more readable, like
|
Good point @danielhuppmann, I've updated the log message, now it looks like this: ERROR nomenclature.processor.required_data:required_data.py:161 Missing required data.
File: tests/data/required_data/required_data/requiredData_apply_error.yaml
Missing rows:
index model scenario variable unit year
0 0 model_a scen_a Primary Energy GWh/yr 2005,2010,2015
1 1 model_a scen_a Primary Energy Mtoe 2005,2010,2015
2 2 model_a scen_b Primary Energy GWh/yr 2005,2010,2015
3 3 model_a scen_b Primary Energy Mtoe 2005,2010,2015
index model scenario variable
0 0 model_a scen_a Final Energy
1 1 model_a scen_b Final Energy
index model scenario variable unit
0 0 model_a scen_a Emissions|CO2 Mt CO2/yr
1 1 model_a scen_b Emissions|CO2 Mt CO2/yr
index model scenario region variable
0 0 model_a scen_a World Final Energy
1 1 model_a scen_b World Final Energy a bit more readable I think. |
If you can remove the index columns, I’ll be 100% happy… |
Ah yeah sure, that wasn't supposed to be there, sorry for that. |
@danielhuppmann cleaned up the output, should be good now: ERROR nomenclature.processor.required_data:required_data.py:161 Missing required data.
File: tests/data/required_data/required_data/requiredData_apply_error.yaml
Missing rows:
model scenario variable unit year
0 model_a scen_a Primary Energy GWh/yr 2005,2010,2015
1 model_a scen_a Primary Energy Mtoe 2005,2010,2015
2 model_a scen_b Primary Energy GWh/yr 2005,2010,2015
3 model_a scen_b Primary Energy Mtoe 2005,2010,2015
model scenario variable
0 model_a scen_a Final Energy
1 model_a scen_b Final Energy
model scenario variable unit
0 model_a scen_a Emissions|CO2 Mt CO2/yr
1 model_a scen_b Emissions|CO2 Mt CO2/yr
model scenario region variable
0 model_a scen_a World Final Energy
1 model_a scen_b World Final 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.
One more suggestion - instead of iterating over the models within the missing_data
assignment, do the loop outside. Then, you could omit the model column from the tables of missing data and instead have it in the log-error message, like
f"Missing required data for model '{model}'."
This would make a more concise output, in particular when we have very long variable names...
Shortened the output, it's now: ERROR nomenclature.processor.required_data:required_data.py:166 Missing required data.
File: tests/data/required_data/required_data/requiredData_apply_error.yaml
Missing for model_a:
scenario variable unit year
0 scen_a Primary Energy GWh/yr 2005,2010,2015
1 scen_a Primary Energy Mtoe 2005,2010,2015
2 scen_b Primary Energy GWh/yr 2005,2010,2015
3 scen_b Primary Energy Mtoe 2005,2010,2015
scenario variable
0 scen_a Final Energy
1 scen_b Final Energy
scenario variable unit
0 scen_a Emissions|CO2 Mt CO2/yr
1 scen_b Emissions|CO2 Mt CO2/yr
scenario region variable
0 scen_a World Final Energy
1 scen_b World Final Energy |
Seems like that last update broke some tests, sorry for that, taking a look now. |
All good now. |
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.
Final suggestion to use single-quotes around the model name, and maybe deactivate py3.11 tests until we can release pyam 2.0
Done |
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.
Thanks!
This PR updates
RequiredDataValidator
to make use of the pyam update torequire_data
and now presents the missing data accurately.Example: