Download script is changed to add all quarters of data - #2115
Download script is changed to add all quarters of data#2115Krishnam24maheshwari wants to merge 8 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Brazil SIDRA IBGE download script, replacing the Selenium-based browser automation with direct API calls using the requests library and reshaping the data into Excel files using pandas. The review feedback recommends improving performance, security, and robustness by using a global requests.Session with connection pooling and retries, re-enabling SSL verification, and adding explicit guard checks to safely parse the API responses.
Added a newline at the end of the file for proper formatting.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Added a robust session setup for HTTP requests with retries and connection pooling. Updated functions to utilize the new session for API calls.
|
Few action items:
|
|
LGTM |
07b0a02 to
c56efe0
Compare
| # Safely create target output directory | ||
| try: | ||
| Path(dest_dir).mkdir(parents=True, exist_ok=True) | ||
| except Exception as e: |
There was a problem hiding this comment.
if download fails, pls fail immediately with an exception and log with sufficent context including the url and parameters. O/w if the download failure is gauranteed to not cause any missing data/deletions, then it is safe to catch.
| """ | ||
| Downloads PNAD Continuous (PNADc) quarterly data directly via IBGE REST API. | ||
| Fetches data starting from Q1 2022 to the latest available period and reshapes | ||
| the output into Excel spreadsheets. |
There was a problem hiding this comment.
why is output in spreadsheet and not csv?
| except Exception as e: | ||
| logging.fatal(f"Failed to rename and move file for {place_name} (Panel {panel_index}): {e}. Exiting script.") | ||
| raise RuntimeError(f"File operation failed: {e}") | ||
| logging.warning(f"Failed to format quarter label for {period_item}: {e}") |
There was a problem hiding this comment.
pls add more context when logging download failures such as url.
download errors can be logging.fatal
| @@ -0,0 +1,24 @@ | |||
| "NumPlaces","NumObservations","Units","StatVar","observationPeriods","MaxValue","MinValue","ScalingFactors","MeasurementMethods","NumObservationsDates","MinDate","MaxDate" | |||
There was a problem hiding this comment.
pls cehck if max and min stay consistent over refreshes, if not pls remove them from goldens
3 participants
This PR resolves the deletions problems caused by 13-quarter rolling window logic as per the API we are using to download data.
Changes Made in brazil_download_script.py:
1. Replaced the 13-quarter rolling window logic with get_available_periods() and different API.
2. Fixed the baseline start period explicitly to 202201 (Q1 2022) onwards through the latest available quarter.
3. Applied this updated period range across all four dataset categories (Average_Real_Income, Mass_Income, Population_Economic_sector, and Employment_And_Unemployment_Labor_Force), preserving all 2022 records and resolving false deletion validation errors.
PR Checklist -
https://docs.google.com/spreadsheets/d/1RD4U94SM2mLcjSZRLjJHVliF737fScBH4rsDU_Pu_hk/edit?gid=1086229089#gid=1086229089