-
Notifications
You must be signed in to change notification settings - Fork 29
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 a first list of Tier 1 variables #57
Conversation
… declared in population/population.yaml
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.
A few suggestions and questions inline. Please capitalize all variables and also the first word in the description.
Co-authored-by: Daniel Huppmann <[email protected]>
Co-authored-by: Daniel Huppmann <[email protected]>
Co-authored-by: Daniel Huppmann <[email protected]>
Co-authored-by: Daniel Huppmann <[email protected]>
Co-authored-by: Daniel Huppmann <[email protected]>
Seems good to be merged @danielhuppmann |
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.
Changes for GDP and population ok
use tag consistently for tiers
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.
Thank you @IAMconsortium/common-definitions-macro-economy for putting together this PR.
Two suggestions before merging, to ensure the long-term viability of the common-definitions variable template: this PR adds quite a few top-level variables (or categories). In order to keep the number of top-level variables small, how about the following:
-
make Employment and Unemployment sub-categories of Population (similar to Population|Urban vs. Population|Rural).
-
make Import and Export sub-categories of Trade and add
[Value]
for disambiguation, so have- Trade|Import [Value]
- Tade|Export [Value]
and then later add
- Trade|Primary Energy|Biomass [Volume]
- ..
Thank you Daniel for reviewing this.
I am ok with this. Even though Employment + Unemployment does not equal total population, but this is a matter of appropriate definition. Olivier and Laurent, if you agree I will push the changes.
This is a good idea. I will think about it. Or we can apply your suggestion and revert it in an upcoming PR. This is something we can discuss during a zoom meeting within the sub-group. |
@FlorianLeblancDr, you mentioned that you will have a Zoom meeting in the sub-group - did you already have a chance to discuss this further? |
Thanks @danielhuppmann for the reminder. I will commit a change for Employment and follow your suggestion Trade, which will help to close the PR faster. Then leave a discussion on GDP composition for later. |
Thanks @FlorianLeblancDr. Agree on your point about population and employment being not quite ideal. How about “Labour Market|Employment” as a new top-level category? |
ping @FlorianLeblancDr - can we move this forward and merge soon? |
…pulation[* variable, adding inactive population for exhaustivity
@danielhuppmann This seems good now :
|
Thank you! I'm not sure whether this list is helpful...
There are too many different categories mixed together... Can I again suggest to use |
Sure, I am kind of agnostic at this point, I was trying to follow your initial suggestion to move it in Population. I find it useful to have the "Not Active" in the tag, in order to suggest, besides definitions, that the sum of Employment + Unemployment is not equal to total population.. even though "Labor Force|Not Active" is a little strange. Feel free to follow your intuition and merge. |
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.
Merging as is and will revise the Labour-Force variables in a follow-up PR.
I open a new pull-request with only Tier 1 variables.
I found it easier for @IAMconsortium/common-definitions-macro-economy to follow discussions and agree with PR with manageable size (working with small subset of variables and from the same tier).
Open to review for @IAMconsortium/common-definitions-macro-economy
@orichters thank for your work with #56 being usefull for the Tier 2 & 3 as well.