Skip to content

S1201 schema fix and data refresh#2110

Open
balit-raibot wants to merge 15 commits into
datacommonsorg:masterfrom
balit-raibot:s1201_schema_fix_and_data_refresh
Open

S1201 schema fix and data refresh#2110
balit-raibot wants to merge 15 commits into
datacommonsorg:masterfrom
balit-raibot:s1201_schema_fix_and_data_refresh

Conversation

@balit-raibot

@balit-raibot balit-raibot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

This PR includes changes:

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the US Census ACS 5-year subject tables scripts and API utilities. Key changes include adding the aiolimiter dependency, clearing overwrite_dcids in the S1201 specification, dynamically importing common column mapping functions to avoid circular imports, ensuring output directories are created, and converting relative imports to absolute imports across the API utilities. Feedback on these changes suggests adding defensive guard checks for dynamic module loading, simplifying directory creation with exist_ok=True, and appending module_dir_ to sys.path to prevent potential ModuleNotFoundError issues when importing the modified API utility modules from other directories.

Comment thread scripts/us_census/acs5yr/subject_tables/s1201/generate_col_map.py Outdated
Comment thread scripts/us_census/acs5yr/subject_tables/s1201/process.py Outdated
Comment thread scripts/us_census/api_utils/census_api_config_fetcher.py Outdated
Comment thread scripts/us_census/api_utils/census_api_data_downloader.py Outdated
@balit-raibot

Copy link
Copy Markdown
Contributor Author

@gemini-code-assists review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates US Census data processing scripts, particularly for the ACS 5-year subject tables (S1201) and API utilities. Key changes include updating statistical variable names to specify age ranges (e.g., adding "15OrMoreYears"), ignoring the "SUMMARY INDICATOR" column in the S1201 specification, casting dataframes to object types before replacing annotations, and refactoring relative imports. The reviewer feedback focuses on improving import robustness by using absolute imports relative to the repository root across multiple files instead of dynamically loading modules, modifying sys.path, or using sibling-level imports. Additionally, the reviewer recommends using a with statement for safe file handling when writing the column map JSON in process.py.

Comment thread scripts/us_census/acs5yr/subject_tables/s1201/generate_col_map.py Outdated
Comment thread scripts/us_census/acs5yr/subject_tables/s1201/process.py Outdated
Comment thread scripts/us_census/api_utils/census_api_config_fetcher.py Outdated
Comment thread scripts/us_census/api_utils/census_api_data_downloader.py Outdated
Comment thread scripts/us_census/api_utils/download_utils.py Outdated
Comment thread scripts/us_census/api_utils/download_utils_test.py Outdated
Comment thread scripts/us_census/api_utils/status_file_utils_test.py Outdated
Comment thread scripts/us_census/api_utils/url_list_compiler.py Outdated
@balit-raibot

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors US Census import scripts and configurations, primarily transitioning relative imports to absolute package imports across api_utils and s1201 modules. It also updates the s1201 specification, test data, and MCF files to use 15OrMoreYears statistical variables instead of overwriting them, adds aiolimiter to requirements, and improves file-writing safety in process.py. Review feedback recommends avoiding brittle relative path traversals (e.g., ../../../../..) when appending to sys.path by dynamically searching for repository markers like .git or WORKSPACE. Additionally, it suggests using absolute imports in process.py to prevent potential namespace collisions.

Comment thread scripts/us_census/acs5yr/subject_tables/s1201/process.py Outdated
Comment thread scripts/us_census/acs5yr/subject_tables/s1201/process.py Outdated
@balit-raibot

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the US Census ACS 5-year subject tables (specifically S1201) and API utilities. Key changes include migrating relative imports to absolute imports, implementing a dynamic project root discovery pattern for Python path resolution, utilizing context managers for file operations, and updating several Statistical Variable DCIDs to explicitly include the "15OrMoreYears" age constraint in both test data and MCF files. Additionally, a pandas type-casting fix was introduced in the API downloader. Feedback suggests applying the robust project root discovery pattern to generate_col_map.py to replace a brittle relative path calculation.

Comment thread scripts/us_census/acs5yr/subject_tables/s1201/generate_col_map.py Outdated
@balit-raibot

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates several US Census scripts to use absolute imports instead of relative imports, adds aiolimiter to the requirements, and refactors directory creation and file writing in process.py. It also updates the statistical variable names in the S1201 test data and MCF files to explicitly include the age range (e.g., 15OrMoreYears). The feedback highlights a redundant os.makedirs call in process.py and recommends replacing wildcard imports with explicit imports in the test files to align with PEP 8 guidelines.

Comment thread scripts/us_census/acs5yr/subject_tables/s1201/process.py Outdated
Comment thread scripts/us_census/api_utils/download_utils_test.py Outdated
Comment thread scripts/us_census/api_utils/status_file_utils_test.py Outdated
@balit-raibot

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates several files in the scripts/us_census directory to improve module import reliability, standardizes column naming conventions in S1201_spec.json and S1201_cleaned.csv, and includes minor code improvements in process.py and census_api_data_downloader.py. The review feedback identified potential runtime errors related to directory existence and missing columns, providing actionable suggestions to improve robustness.

Comment thread scripts/us_census/acs5yr/subject_tables/s1201/process.py
Comment thread scripts/us_census/api_utils/census_api_data_downloader.py
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.

1 participant