Skip to content
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

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

Merged
merged 15 commits into from
Jun 11, 2024

Conversation

jdebacker
Copy link
Member

@jdebacker jdebacker commented May 31, 2024

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 Reduce API calls in demographics.py #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 Update demographics.py to use the new UN API format #931).
  3. Add functionality to read in cached files, pulling the years of data one wants from them. [NOT YET STARTED]

@jdebacker
Copy link
Member Author

To be resolved:

  • Does the prompt for a token work from a Jupyter Notebook?
  • Does the prompt for a token work when running unit tests locally?
  • Is there a way Github Actions can supply the token as a GH secret when running unit tests on GH Actions?
  • What is the best way to allow users to separate run functions (e.g., get_fert or get_mort) and supply their token to the API? Current changes are to just ask for it when one pings get_pop_obj and that call requires fetching data from the UN.

@jdebacker
Copy link
Member Author

jdebacker commented May 31, 2024

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
Copy link
Member Author

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
Copy link

codecov-commenter commented May 31, 2024

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

@@            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 Coverage Δ
unittests 71.92% <31.11%> (-0.52%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
ogcore/__init__.py 100.00% <100.00%> (ø)
ogcore/txfunc.py 30.62% <ø> (+0.04%) ⬆️
ogcore/demographics.py 56.88% <29.54%> (-4.46%) ⬇️

... and 1 file with indirect coverage changes

@jdebacker
Copy link
Member Author

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

  • User is prompted to enter token, any token entered is saved to a file, un_api_token.txt for future use.
  • If working token is entered, data is fetched from UN WPP data API
  • If token entered is incorrect, or if any other issue connecting to UN WPP data API, data is downloaded from the EAPD Population-Data repo..

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

@SeaCelo
Copy link

SeaCelo commented Jun 7, 2024

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

@jdebacker jdebacker marked this pull request as ready for review June 7, 2024 22:02
@rickecon
Copy link
Member

@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.

@rickecon rickecon merged commit 4e6ae04 into PSLmodels:master Jun 11, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants