Skip to content

automated this import - #2221

Open
niveditasing wants to merge 3 commits into
datacommonsorg:masterfrom
niveditasing:UsCensusGeo_surfacearea_automation
Open

niveditasing wants to merge 3 commits into
datacommonsorg:masterfrom
niveditasing:UsCensusGeo_surfacearea_automation

Conversation

@niveditasing

@niveditasing niveditasing commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

Addressed Gemini review comments on the US Census Surface Area import:

  1. Fixed fallback file matching (preprocess.py): Updated the fallback logic so it won't accidentally match files from other years that have a year prefix (e.g. 2018_...).
  2. Fixed file moving on Windows (preprocess.py): Closed the temporary download file before moving it to destination to prevent PermissionError.
  3. Added unit tests (preprocess_test.py): Added test cases for fallback matching and atomic file downloads.
  4. Cleaned up config and docs (manifest.json, README.md): Removed unused golden_data references from manifest.json and aligned README.md with the active rules in validation_config.json.

Testing

@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 automates the ingestion and preprocessing of US Census Gazetteer files and State Area Measurements to calculate surface area across various geographic entities. Feedback on the changes highlights a potential bug in the fallback file-matching logic that could lead to silent data corruption, a cross-platform compatibility issue on Windows due to moving an open temporary file, and a discrepancy where the validation configuration is missing two of the rules documented in the README.

Comment thread scripts/us_census/surface_area/preprocess.py
Comment thread scripts/us_census/surface_area/preprocess.py
Comment thread scripts/us_census/surface_area/validation_config.json
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.

2 participants