PSLmodels / OG-Core

An overlapping generations model framework for evaluating fiscal policies.
https://pslmodels.github.io/OG-Core/
Creative Commons Zero v1.0 Universal
68 stars 119 forks source link

Updates to `demographics.py` for changes in UN API #936

Closed jdebacker closed 5 months ago

jdebacker commented 5 months ago

This PR updates the demographics.py module for updates in the UN WPP data portal API, which now requires users to authenticate with an API token.

The PR: 1) Changes the calls to the API to download multiple years at a time, which significantly reduces the number of API calls (Issue #935) 2) Has demographics.py look for a file named un_api_token.txt in the current directory, and if not there, prompts the user to enter their API token in the command line (Issue #931). 3) Add functionality to read in cached files, pulling the years of data one wants from them. [NOT YET STARTED]

jdebacker commented 5 months ago

To be resolved:

jdebacker commented 5 months ago

One idea is to include:

global UN_TOKEN
# Check for a file named "un_api_token.txt" in the current directory
if os.path.exists(os.path.join("un_api_token.txt")):
    with open(os.path.join("un_api_token.txt"), "r") as file:
         UN_TOKEN = file.read().strip()
else:  # if file not exist, prompt user for token
     UN_TOKEN = input("Please enter your UN API token: ")

in get_un_data, but have a command following the user input prompt to save the token to a file named un_api_token.txt. This way, on subsequent calls to the API, the token is just read from a file.

Would there be any security concerns with this? e.g., a user's token saved on a device that isn't theirs?

To solve that, we can have a separate prompt, for "save token to disk" and only write to disk if yes...

jdebacker commented 5 months ago

For the ability to use cached files and change the start/end year, we need to save the year as a variables to the output files (fert_rates.csv, etc.)

codecov-commenter commented 5 months ago

Codecov Report

Attention: Patch coverage is 31.11111% with 31 lines in your changes missing coverage. Please review.

Project coverage is 71.92%. Comparing base (e762b06) to head (138a981).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936/graphs/tree.svg?width=650&height=150&src=pr&token=98mQCVhspd&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels)](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) ```diff @@ Coverage Diff @@ ## master #936 +/- ## ========================================== - Coverage 72.44% 71.92% -0.52% ========================================== Files 19 19 Lines 4696 4716 +20 ========================================== - Hits 3402 3392 -10 - Misses 1294 1324 +30 ``` | [Flag](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) | `71.92% <31.11%> (-0.52%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) | Coverage Δ | | |---|---|---| | [ogcore/\_\_init\_\_.py](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936?src=pr&el=tree&filepath=ogcore%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels#diff-b2djb3JlL19faW5pdF9fLnB5) | `100.00% <100.00%> (ø)` | | | [ogcore/txfunc.py](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936?src=pr&el=tree&filepath=ogcore%2Ftxfunc.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels#diff-b2djb3JlL3R4ZnVuYy5weQ==) | `30.62% <ø> (+0.04%)` | :arrow_up: | | [ogcore/demographics.py](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936?src=pr&el=tree&filepath=ogcore%2Fdemographics.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels#diff-b2djb3JlL2RlbW9ncmFwaGljcy5weQ==) | `56.88% <29.54%> (-4.46%)` | :arrow_down: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/936/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels)
jdebacker commented 5 months ago

This PR now handles UN data access in the following way:

@rickecon @SeaCelo, let me know if the changes in demographics.py look good to you.

SeaCelo commented 5 months ago

@jdebacker This is a sensible approach. I'll test it and see if there are any issues

rickecon commented 5 months ago

@jdebacker. This looks great. Merging as soon as tests pass. I just committed one extra change at the end of CHANGELOG.md that adds the hyperlink.