Skip to content

Mongolia_Imports Fix - #2167

Open
kartik-s21 wants to merge 17 commits into
datacommonsorg:masterfrom
kartik-s21:mongolia-hardening
Open

Mongolia_Imports Fix#2167
kartik-s21 wants to merge 17 commits into
datacommonsorg:masterfrom
kartik-s21:mongolia-hardening

Conversation

@kartik-s21

@kartik-s21 kartik-s21 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Description

Builds on top of the initial Mongolia import fixes (#2166) by completing the remaining hardening requirements across Mongolia Health, Education, Demographics, and Employment:

  1. Summary-Report Goldens & Validations:

    • Generated golden_data/input<N>_golden_summary_report.csv for each manifest input across all 4 imports using tools/import_validation/validator_goldens.py with standard property sets.
    • Configured validation_config.json with GOLDENS_CHECK rules for every input and linked validation_config_file in manifests.
  2. Manifest node_mcf Wildcards:

    • Added "node_mcf": "output_files/<prefix>*.mcf" for all manifest inputs to ensure processor-generated StatVar and schema MCF files are properly ingested by the import pipeline.
  3. Test Data Fixtures & Unit Test Suites:

    • Added small focused test input fixtures (testdata/<prefix>_input.csv) covering all regression cases (hospital bed classifications, qualified teachers, consolidated professional field StatVars, and 15OrMoreYears demographics).
    • Generated expected .csv, .tmcf, _stat_vars.mcf, and _stat_vars_schema.mcf fixtures in each domain's testdata/.
    • Created test suites health_test.py, education_test.py, demographics_test.py, employment_test.py with reusable base testing utilities in mongolia_test_helper.py.

Verification

  • Ran python3 -m unittest for all 4 test suites — All 25 unit tests passed.
  • Validated all 25 summary reports against their respective goldens using validator_goldens.py100% matched.
  • Code_Review_Agent Report : link

Cloud Batch Jobs (test_env) :
Mongolia_Education - link
Mongolia_Health - link
Mongolia_Demographics - link
Mongolia_Employment - link

rohitkumarbhagat and others added 11 commits August 18, 2026 19:03
… unit tests

- Added summary-report golden files and GOLDENS_CHECK validation rules in validation_config.json for all 25 inputs across Health, Education, Demographics, and Employment.
- Declared 'output_files/<prefix>*.mcf' node_mcf wildcards in all manifest inputs to publish newly generated StatVars and schema nodes.
- Added comprehensive unit test suites (health_test.py, education_test.py, demographics_test.py, employment_test.py) and test fixtures covering all regression cases.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

The number of changes in this pull request is too large for Gemini Code Assist to generate a review.

- Generated input<N>_golden_observations.csv for observationAbout across all 25 inputs in Health, Education, Demographics, and Employment using top 100k places.
- Added GOLDENS_CHECK rules for golden observations in validation_config.json for each domain.

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.

Widespread Unit Test Failures Due to Unmapped Regional Places & Leading Whitespace

  • Files:
    • statvar_imports/mongolia_imports/mongolia_health/health_test.py (all 6 tests fail)
    • statvar_imports/mongolia_imports/mongolia_employment/employment_test.py (test_labour_underutilization, test_registered_unemployed_by_education_level fail)
    • statvar_imports/mongolia_imports/mongolia_demographics/demographics_test.py (test_mid_year_total_population_by_region, test_number_of_households_by_region fail)
  • Problem:
    • Input CSVs contain regional aggregate rows with leading spaces (e.g., " Western region", " Khangai region", " Central region", " Eastern region"). Because these strings are neither ignored in the pvmap nor present in mongolia_place_resolver.csv, stat_var_processor.py fails with a fatal error.
    • In employment input files, the region string has a typo "Centrel" instead of "Central".
  • Fix: Add explicit #ignore,skip mappings for regional aggregates in the corresponding pvmap CSVs (e.g. Location: Western region,#ignore,skip, Location:Centrel,#ignore,skip), or map them in mongolia_place_resolver.csv if they represent valid entities.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

already fixed.

},
{
"rule_id": "check_goldens_summary_report_input1",
"description": "Validates summary_report.csv for input1 (infant_mortality_per_1000_live_births_by_month_region) against its golden summary data",

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.

2. Network Dependency / GCS Fallback in Unit Test Helper

  • File: statvar_imports/mongolia_imports/mongolia_test_helper.py:23
  • Problem:
    self.existing_statvar_mcf = '/tmp/stat_vars.mcf' if os.path.exists(
        '/tmp/stat_vars.mcf'
    ) else 'gs://unresolved_mcf/scripts/statvar/stat_vars.mcf'
    Defaulting to gs://unresolved_mcf/... makes unit tests non-hermetic. In sandboxed CI/CD or local test environments without GCS credentials, tests fail immediately.
  • Fix: Use a local dummy schema file or a checked-in fixture instead of defaulting to a live GCS bucket.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Updated mongolia_test_helper.py to use a local fallback (/dev/null) when /tmp/stat_vars.mcf is not present, ensuring unit tests runs 100% offline without requiring GCS access.

"input_files": "../../input1/genmcf/summary_report.csv"
}
},
{

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.

3. Fragile compare_mcf_files Assertion for Existing Standard Statvars

  • File: statvar_imports/mongolia_imports/mongolia_test_helper.py:30, 100-104
  • Problem:
    stat_var_processor does not create *_stat_vars.mcf when all generated statvars already exist (e.g. Count_Household, Count_Person_Urban). Because expected MCF files were checked into testdata/, compare_mcf_files fails with:
    AssertionError: Actual MCF file was not created: ..._output_stat_vars.mcf
  • Fix: Either remove expected *_stat_vars.mcf from testdata/ for tables with no new statvars, or adjust compare_mcf_files to verify that both expected and actual are empty.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

already fixed.

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.

4. Stale Golden CSV for Marital Status Test

  • File: statvar_imports/mongolia_imports/mongolia_demographics/testdata/resident_population_by_agegroup_15_and_over_and_maritalstatus_output.csv
  • Problem:
    resident_population_by_agegroup_15_and_over_and_maritalstatus_pvmap.csv was updated to add gender mappings and Age group:Total,age,[15 - Years], generating statvars with [15 - Years] (e.g. Count_Person_15OrMoreYears_Male_NowMarried). However, the test output CSV was not regenerated, causing a 28-row diff in demographics_test.py.
  • Fix: Regenerate and commit resident_population_by_agegroup_15_and_over_and_maritalstatus_output.csv.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

already fixed.

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.

5. Manifest source_files Missing golden_data/*.csv

  • Files: manifest.json in mongolia_demographics, mongolia_education, mongolia_employment, mongolia_health
  • Problem: source_files only includes counters/*.csv and input_files/*.csv. Per Data Commons import guidelines, "golden_data/*.csv" must be listed under source_files so validation summary reports are archived to GCS for automated validation refresh runs.
  • Fix:
    "source_files": [
      "counters/*.csv",
      "golden_data/*.csv",
      "input_files/*.csv"
    ]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Added "golden_data/*.csv" to source_files across all 4 manifest.json files.

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.

6. Missing Date Consistency & Freshness Rules in validation_config.json

  • Files: validation_config.json in all 4 import directories
  • Problem: Only check_deleted_records_percent and check_goldens_summary_report_input<N> are specified. Date consistency (MAX_DATE_CONSISTENT) and date freshness (MAX_DATE_LATEST / SQL checks) validation rules are omitted.
  • Fix: Add MAX_DATE_CONSISTENT and appropriate date freshness validation checks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We are keeping the deletion threshold at the default 0.1 value, and we are not adding MAX_DATE_CONSISTENT for now as it is not needed at this stage. If date consistency or freshness rules are required in the future, we can add them in a subsequent update.

Comment thread statvar_imports/mongolia_imports/common_download_script.py
except Exception as e:
error_msg = f"FATAL ERROR: Failed to download from {url}: {e}"
logging.fatal(error_msg)
raise RuntimeError(error_msg)

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.

8. Redundant raise After logging.fatal

  • File: statvar_imports/mongolia_imports/common_download_script.py:108, 115
  • Problem: Under absl.app.run(), logging.fatal(error_msg) terminates execution immediately, making the subsequent raise RuntimeError(error_msg) redundant/unreachable.
  • Fix: Use logging.error(error_msg) before raise, or rely exclusively on logging.fatal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Replaced logging.fatal with logging.error before raise RuntimeError in common_download_script.py.

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.

can you decrease the test size for these files to be small

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Created a Report for the changes done - https://paste.googleplex.com/5301467647967232

- Made unit test helper hermetic by using local fallback when /tmp/stat_vars.mcf is absent (Comment 2).
- Added golden_data/*.csv to source_files across all 4 manifests (Comment 5).
- Added EMPLOYMENT_TABLES to common_download_script.py for unified downloads (Comment 7).
- Replaced logging.fatal with logging.error before raise in common_download_script.py (Comment 8).
…hermetic

- Trim test fixture input CSVs to 2 representative time periods and 4 sample rows
- Regenerate expected CSV, TMCF, and MCF test outputs across education, health, demographics, and employment
- Update mongolia_test_helper.py to default existing_statvar_mcf to /dev/null for reproducible hermetic testing
- Reduce testdata storage footprint by ~96.1% (from 1.04MB to ~40KB, removing 9,300+ lines) while maintaining 100% test coverage (25/25 tests passing)

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.

For imports using StatVarProcessor, do not write unit tests that run StatVarProcessor; however, do create test_data/ fixtures with sample inputs and expected outputs (to be used later for E2E tests).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done! I have updated the PR accordingly:

  • Removed StatVarProcessor unit tests: Deleted the unit test suites that were executing StatVarProcessor.
  • Created downsized test_data/ fixtures: Added standard test_data/ directories with sample input CSVs and expected output CSV, TMCF, and MCF files for all 4 imports (mongolia_demographics, mongolia_education, mongolia_health, mongolia_employment). All fixtures are downsized to minimal slices (~42.7 KB total across all 25 tables) to keep the repository lightweight for future E2E testing.
  • Retained download unit test: Maintained common_download_script_test.py to test custom NSO API download logic, URL configurations, and payload formulation using hermetic mocked responses.

"rules": [
{
"rule_id": "check_deleted_records_percent",
"description": "Strictly enforce historical deletion average threshold of 10%",

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.

1. Fix DELETED_RECORDS_PERCENT Threshold in validation_config.json

In all 4 validation_config.json files, the rule description states 10% tolerance, but "threshold": 0.1 is set.

pls correct the comment or value.

2. (Recommended) Add Date Validation Rules

Add MAX_DATE_CONSISTENT and MAX_DATE_LATEST (or allowable-lag SQL_VALIDATOR rules) to validation_config.json so automated weekly cron runs catch stale source feeds.

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