[Nde-Dilan]

• Adds include_aggregates (and alias exclude_aggregates), include_non_sovereign, and sovereign_only flags to get_dataframe, get_series, get_data, and get_countries.
• Supports country="countries" and country="sovereign" string shortcuts.
• Exports NON_SOVEREIGN_COUNTRIES / non_sovereign_countries at package level.
• Full test suite passes with 100% backward compatibility maintained.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Nde-Dilan]

@OliverSherouse While working with the package I found myself doing these operations again and again, after checking the issue list, I saw that discussion was already on its way. So here is my contribution to the discussion, let me know if anything needs to change.

[OliverSherouse]

@Nde-Dilan Thanks for the PR! Where do these lists come from? I'm not super high on the idea of hard-coding lists of countries in the package; correctness should not, ideally, rely on my diligence in making updates or the users in installing them. Is there a way to get this information programatically from the API?

[Nde-Dilan]

Hello @OliverSherouse , I did research on a way to get it directly from the API, but found no existing method.

The list is coming from the official Member states of the united nations found here.

And yes, having to update that manually isn't really ideal, even though one could argue sovereign countries won't be changing every now and then :).

[OliverSherouse]

Well, good news and bad news. We can detect aggregates by looking at the country's region, so I'm totally happy to have a convenience argument for that. The way I think it should go is to have an exclude_aggregates in the get_countries function. Then, for the data-fetching functions, we have a magic value country='exclude_aggregates' that tells the function to call get_countries with that parameter. I don't want to have the parameter itself on the data functions because it's only meaningful when country is all and these signatures don't need to get any longer. I think having that in here would be a good improvement and I'm a little surprised I never did this myself because I certainly thought about it.

I don't think I want to include a constant of non-sovereign countries. I can totally see how this is a reasonable use case for you, but we'd be importing concepts that aren't native to the World Bank system and I would then be placed in the unlovely position of deciding questions like when and to what extent Palestine is a state. I also think users may actually have a lot of very slightly different ways to decide which set of countries they're interested in and I don't want to privilege any one that isn't reflected in the API itself.

[Nde-Dilan]

I clearly understand your point of view, and I am even starting to think that might be the reason why it's not yet added to the world bank API directly, not everyone might agree with what the United Nations is defining as sovereign country.

Then I think the best way to approach it right now is just to leave that task to the developer using the package and depending on its use case he/she might handle it differently.

[OliverSherouse]

Did you still want to add in the aggregates?

[Nde-Dilan]

Hello @OliverSherouse , I have removed the constant of countries and added the detection of aggregates via region, with a single magic value country="exclude_aggregates" in data fetching functions.