Add a countries attribute to the region codelist
#374
Conversation
I am on board with that suggestion. We'll just might have to update some instance workflow repositories that are currently using the |
| """Verifies that each country name is defined in `nomenclature.countries`.""" | ||
| if invalid_country_names := set(v) - set(countries.names): | ||
| raise ValueError( | ||
| f"Region '{info.data['name']}' uses non-standard country name(s): " |
There was a problem hiding this comment.
I'd reference the exact ISO norm that we're using, just "standard" is a little ambiguous in my opinion:
| f"Region '{info.data['name']}' uses non-standard country name(s): " | |
| f"Region '{info.data['name']}' uses non-ISO3166-1 compliant country name(s): " |
There was a problem hiding this comment.
We do not strictly follow ISO-3166-1, we use our own adaptation, see
https://github.com/IAMconsortium/nomenclature/blob/main/nomenclature/countries.py.
I added a link to the documentation for clarity.
| @@ -4,6 +4,5 @@ repositories: | |||
| definitions: | |||
| region: | |||
| repository: common-definitions | |||
| country: true | |||
There was a problem hiding this comment.
As an alternative you could leave this in and use the pin feature to pin this config to the latest pre-G20 common-definitions revision:
repositories:
common-definitions:
url: https://github.com/IAMconsortium/common-definitions.git/
hash: 4f77be8
definitions:
region:
repository: common-definitions
country: true
variable:
repository: common-definitionsThere was a problem hiding this comment.
We do have a separate test for country: true, so I'd rather keep the tests consistent with the latest version of common-definitions to be warned about future conflicts early. There isn't so much extra benefit to the joint test of common-definitions and country: true.
| @@ -5,6 +5,5 @@ repositories: | |||
| definitions: | |||
| region: | |||
| repository: common-definitions | |||
| country: true | |||
There was a problem hiding this comment.
Same as above, to fix the tests you could pin common-definitions:
repositories:
common-definitions:
url: https://github.com/IAMconsortium/common-definitions.git/
hash: 4f77be8
definitions:
region:
repository: common-definitions
country: true
variable:
repository: common-definitions| @@ -51,12 +51,6 @@ def test_definition_from_general_config(workflow_folder): | |||
| assert "Region A" in obs.region | |||
| # imported from https://github.com/IAMconsortium/common-definitions repo | |||
| assert "World" in obs.region | |||
| # added via general-config definitions | |||
| assert "Austria" in obs.region | |||
There was a problem hiding this comment.
If you introduce the pin for common-definitions, you can bring this one back (I think)
While working on model-registrations for common-definitions, I noticed that we've not been consistent whether to add a list of countries or ISO3-codes to clearly define model-native regions.
In favor of readability, I'd prefer to use country-names, see for example IAMconsortium/common-definitions#118.
This PR adds an optional
countriesattribute to the RegionCode class, and validates any names in this list against the nomenclature.countries list.As a follow-up PR, I suggest that we deprecate the
iso3_codesattribute and instead add analpha_3property that returns the countries-elements translated to ISO3 codes. This way, we have good readability and consistency in the GitHub repos and anyone still prefering to work with ISO3 codes can still comfortably work with those.