Add node code lists R17 and R20

[SiddharthJoshi-Git]

This PR adds two new regional mappings for GUIDE project and YSSP'23 work related to it. The R17 regional mapping is for YSSP work and R20 for overall GUIDE project.

How to review

• Read the diff and note that the CI checks all pass.
• Confirm the new lists appear in the documentation.
• More TBA.

PR checklist

• [x] Continuous integration checks all ✅

• [x] Add or expand tests; coverage checks both ✅
• [x] Add, expand, or update documentation.

• [x] Update doc/whatsnew.

[SiddharthJoshi-Git]

Hi @khaeru

I opened a new #PR109 which superseded #PR108. The issue with licensing error from GAMS is now resolved, but the tests are fialing due to hardcoded list of "expected" node codelist within the/data/nodefolder. Especially this message_ix_models/tests/model/test_structure.py. Now the question is if I modify the test_structure.py file to include the new node code lists within this PR, will the test pass?

[khaeru]

Now the question is if I modify the test_structure.py file to include the new node code lists within this PR, will the test pass?

Yes, you should try to update the test. You can either push a commit with your change and see what happens here, or you can try to run the test locally before committing and pushing, using e.g. pytest -k codelists.

I would also ask that you update the test_nodes() function in the same file with cases that check that contents of the two new code lists.

[codecov[bot]]

Codecov Report

Merging #109 (93fb983) into main (43aa32b) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 93fb983 differs from pull request most recent head 988134a. Consider uploading reports for the commit 988134a to get more accurate results

@@          Coverage Diff          @@
##            main    #109   +/-   ##
=====================================
  Coverage   68.3%   68.3%           
=====================================
  Files         63      63           
  Lines       4174    4174           
=====================================
  Hits        2855    2855           
  Misses      1319    1319           

Files Changed	Coverage Δ
message_ix_models/tests/model/test_structure.py	100.0% <ø> (ø)

[SiddharthJoshi-Git]

Hi @khaeru,

It seems that the PR passed all the checks after modifications. The only thing left is to update the docs for node code list. Is this done automatically or do I need to manually update the links on this page?

https://docs.messageix.org/projects/models/en/stable/pkg-data/node.html#node-code-lists

[glatterf42]

Hi @SiddharthJoshi-Git, it looks to me like you have to edit doc/pkg-data/node.rst manually, but (following the other files), including all data from your R17.yaml file shouldn't take more than a few lines.

[khaeru]

The only thing left is to update the docs for node code list.

Please re-read above:

I would also ask that you update the test_nodes() function in the same file with cases that check that contents of the two new code lists.

[SiddharthJoshi-Git]

Hi @SiddharthJoshi-Git, it looks to me like you have to edit doc/pkg-data/node.rst manually, but (following the other files), including all data from your R17.yaml file shouldn't take more than a few lines.

Thanks @glatterf42, I will update this today manually.

[SiddharthJoshi-Git]

The only thing left is to update the docs for node code list.

Please re-read above:

I would also ask that you update the test_nodes() function in the same file with cases that check that contents of the two new code lists.

Many thanks @khaeru . I have now added a new commit https://github.com/iiasa/message-ix-models/pull/109/commits/6729d9faf467d841dfbb08d99ca7c4998f726ac4 which parametrises checks for test_node() function. I hope this concludes the PR checks.

Based on @glatterf42 comment Hi @SiddharthJoshi-Git, it looks to me like you have to edit [doc/pkg-data/node.rst](https://github.com/iiasa/message-ix-models/blob/main/doc/pkg-data/node.rst) manually, but (following the other files), including all data from your R17.yaml file shouldn't take more than a few lines.

I will now proceed to add the node code lists to the .rst file

[SiddharthJoshi-Git]

Dear @khaeru, @glatterf42

The PR is now ready for merging.

Many thanks for your time.

Best, Sid

[khaeru]

@SiddharthJoshi-Git thanks! At least a couple things remain:

• Please see the code style that applies to all the packages in our message_ix stack, particularly the "7 rules of a great Git commit message". If you like, you can e.g. git rebase --interactive HEAD~8 on your local branch and use the r/reword command to apply this style to the commit messages. Otherwise, the PR can eventually be merged by the "squash-and-merge" method, i.e. as a single commit.
• Please check the boxes in the "PR checklist" in the template above if the things described are done. At a glance, it seems like you have not updated doc/whatsnew; see iiasa/message-ix-models@b275f4d for a similar example.

[SiddharthJoshi-Git]

@SiddharthJoshi-Git thanks! At least a couple things remain:

• Please see the code style that applies to all the packages in our message_ix stack, particularly the "7 rules of a great Git commit message". If you like, you can e.g. git rebase --interactive HEAD~8 on your local branch and use the r/reword command to apply this style to the commit messages. Otherwise, the PR can eventually be merged by the "squash-and-merge" method, i.e. as a single commit.
• Please check the boxes in the "PR checklist" in the template above if the things described are done. At a glance, it seems like you have not updated doc/whatsnew; see b275f4d for a similar example.

Many Thanks @khaeru,

For suggestion 1, I am ok with squash and merge strategy as the atomic commits on this PR are just for verification and not required after merge. Additionally, I must admit that I have not donerebase/reword before and hence I am afraid that I will mess-up something along the way. I have taken note of the resource "7 rules of a great Git commit message" and will apply the rules on next PR.

For suggestion 2, I have modified the whatsnew.rst via af04b2d