[Brazil_SIDRA_IBGE] Download script is changed to add all quarters of data - #2115
[Brazil_SIDRA_IBGE] Download script is changed to add all quarters of data#2115Krishnam24maheshwari wants to merge 12 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
…e instant debugging and validate golden checks
Krishnam24maheshwari
changed the title
| } | ||
| }, | ||
| { | ||
| "rule_id": "Check_goldens_output_csv_population_economic", |
There was a problem hiding this comment.
rename all the rules to begin with lower case "C"
| @@ -0,0 +1,34 @@ | |||
| "observationAbout" | |||
There was a problem hiding this comment.
Let us create a single observation file if all the files are having same set of places
| 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}") | ||
| err_msg = f"Unexpected error when retrieving available periods. URL: {url}, Error: {e}" | ||
| logging.fatal(err_msg) |
There was a problem hiding this comment.
Lets not use logging.fatal. Rather use logging.error (err_msg)
| os.makedirs(os.path.join(DOWNLOAD_DIR, folder_name), exist_ok=True) | ||
| except Exception as dir_err: | ||
| err_msg = f"Could not setup target download folders in '{DOWNLOAD_DIR}': {dir_err}" | ||
| logging.fatal(err_msg) |
There was a problem hiding this comment.
Please replace all occurrences of logging.fatal with logging.error
| except ValueError as json_err: | ||
| err_msg = f"Failed to parse JSON period metadata response. URL: {url}, Error: {json_err}" | ||
| logging.fatal(err_msg) | ||
| raise RuntimeError(err_msg) from json_err |
There was a problem hiding this comment.
Lets us simply use "raise" to raise the error and stop the program instead of "raise RuntimeError(err_msg) from json_err"
| except TimeoutException: | ||
| logging.fatal("Timeout while waiting for the place selection dropdown. Exiting script.") | ||
| raise RuntimeError("Could not find the 'codigolist-pnadct' element.") | ||
| Path(dest_dir).mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
Lets use the command os.makedirs to maintain consistency
| if __name__ == "__main__": | ||
| flags.FLAGS.log_dir = SCRIPT_DIR | ||
| app.run(main) | ||
| try: |
There was a problem hiding this comment.
This try except block is redundant
| """ | ||
| 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.
Are we still using Excel spreadsheets? If not, lets change the comment
|
|
||
| # Safely write spreadsheet out to file | ||
| try: | ||
| with pd.ExcelWriter(filepath, engine='openpyxl') as writer: |
There was a problem hiding this comment.
Lets out put all files as CSV rather than Excel
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.
Added goldens rules (golden_observations.csv) goldens_summary_report is available for only population_economic file because non of the statvars generated in other imports is there in "gs://unresolved_mcf/import_validation/nl_statvars.csv"