Skip to content

Migrate compatible imports to use download_util_script.py - #2188

Open
ajaits wants to merge 9 commits into
datacommonsorg:masterfrom
ajaits:dc-url
Open

Migrate compatible imports to use download_util_script.py#2188
ajaits wants to merge 9 commits into
datacommonsorg:masterfrom
ajaits:dc-url

Conversation

@ajaits

@ajaits ajaits commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Migrates compatible data imports to use the shared util/download_util_script.py utility.

@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 migrates several import manifests to use the centralized download_util_script.py utility instead of custom download scripts, and changes a configuration setting in configs.py. The reviewer identified several critical path mismatches in the manifests where import_inputs and source_files were not updated to reflect the new output directories of the download and processing scripts. Additionally, the reviewer requested reverting the unrelated configuration change in configs.py to keep the pull request focused.

Comment on lines +18 to +29
"import_inputs": [
{
"import_name": "BEA_USStatesQuarterlyGDP",
"curator_emails": [
"support@datacommons.org"
],
"provenance_url": "https://www.bea.gov/data/gdp/gdp-state",
"provenance_description": "Quarterly data on inflation-adjusted GDP at a state level in the United States from the US Bureau of Economic Analysis.",
"scripts": [
"common_download.py",
"import_data.py",
"import_industry_data_and_gen_mcf.py"
],
"source_files": [
"input_data/*.csv"
],
"import_inputs": [
{
"cleaned_csv": "states_gdp.csv",
"template_mcf": "states_gdp.tmcf",
"node_mcf": "states_gdp_statvars.mcf"
},
{
"cleaned_csv": "states_industry_gdp.csv",
"node_mcf": "states_gdp_industry_statvars.mcf",
"template_mcf": "states_industry_gdp.tmcf"
}
],
"cron_schedule": "30 10 30 1,4,7,10,12 *"
"cleaned_csv": "states_gdp.csv",
"template_mcf": "states_gdp.tmcf",
"node_mcf": "states_gdp_statvars.mcf"
},
{
"cleaned_csv": "states_industry_gdp.csv",
"node_mcf": "states_gdp_industry_statvars.mcf",
"template_mcf": "states_industry_gdp.tmcf"
}
]
],

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.

critical

There is a critical mismatch between the stat_var_processor.py output path (output_files/BEA_GDP_output) and the expected files in import_inputs (which still point to the old states_gdp.csv and states_industry_gdp.csv files). Since the new processor generates a single set of output files under output_files/BEA_GDP_output, import_inputs must be updated to point to these new files so the executor can find them.

      "import_inputs": [
        {
          "cleaned_csv": "output_files/BEA_GDP_output.csv",
          "template_mcf": "output_files/BEA_GDP_output.tmcf",
          "node_mcf": "output_files/BEA_GDP_output_stat_vars.mcf"
        }
      ],

Comment on lines +18 to +23
"import_inputs": [
{
"import_name": "US_CCD_Enrollment",
"curator_emails": [
"support@datacommons.org"
],
"provenance_url": "https://nces.ed.gov/programs/digest/d24/tables/dt24_203.65.asp",
"provenance_description": "Enrollment in public elementary and secondary schools, by level, grade, and race/ethnicity.",
"scripts": [
"download.py",
"../../../tools/statvar_importer/stat_var_processor.py --input_data=input_files/ccd_enrollment_input.csv --pv_map=CCD_Enrollment_pvmap.csv --config_file=CCD_Enrollment_metadata.csv --output_path=output/CCD_Enrollment_output --existing_statvar_mcf=gs://unresolved_mcf/scripts/statvar/stat_vars.mcf"
],
"source_files": [
"input_files/ccd_enrollment_input.csv"
],
"import_inputs": [
{
"template_mcf": "output/CCD_Enrollment_output.tmcf",
"cleaned_csv": "output/CCD_Enrollment_output.csv"
}
],
"cron_schedule": "0 06 * * 5"
"template_mcf": "output/CCD_Enrollment_output.tmcf",
"cleaned_csv": "output/CCD_Enrollment_output.csv"
}
]
} No newline at end of file
],

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.

critical

There is a critical mismatch between the stat_var_processor.py output path (output_files/enrollment_output) and the expected files in import_inputs (which still point to output/CCD_Enrollment_output.tmcf and output/CCD_Enrollment_output.csv). import_inputs must be updated to point to the new output files under output_files/enrollment_output so the executor can find them.

Suggested change
"import_inputs": [
{
"import_name": "US_CCD_Enrollment",
"curator_emails": [
"support@datacommons.org"
],
"provenance_url": "https://nces.ed.gov/programs/digest/d24/tables/dt24_203.65.asp",
"provenance_description": "Enrollment in public elementary and secondary schools, by level, grade, and race/ethnicity.",
"scripts": [
"download.py",
"../../../tools/statvar_importer/stat_var_processor.py --input_data=input_files/ccd_enrollment_input.csv --pv_map=CCD_Enrollment_pvmap.csv --config_file=CCD_Enrollment_metadata.csv --output_path=output/CCD_Enrollment_output --existing_statvar_mcf=gs://unresolved_mcf/scripts/statvar/stat_vars.mcf"
],
"source_files": [
"input_files/ccd_enrollment_input.csv"
],
"import_inputs": [
{
"template_mcf": "output/CCD_Enrollment_output.tmcf",
"cleaned_csv": "output/CCD_Enrollment_output.csv"
}
],
"cron_schedule": "0 06 * * 5"
"template_mcf": "output/CCD_Enrollment_output.tmcf",
"cleaned_csv": "output/CCD_Enrollment_output.csv"
}
]
}
\ No newline at end of file
],
"import_inputs": [
{
"template_mcf": "output_files/enrollment_output.tmcf",
"cleaned_csv": "output_files/enrollment_output.csv"
}
],

})
# API Environment used by the import tool
use_autopush_dc_api: bool = True
use_autopush_dc_api: bool = False

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.

medium

This configuration change (use_autopush_dc_api: bool = False) is unrelated to the PR's objective of migrating compatible imports to use download_util_script.py. Unrelated changes should be kept out of this PR to keep it focused and avoid accidental environment switches.

Suggested change
use_autopush_dc_api: bool = False
use_autopush_dc_api: bool = True
References
  1. Keep pull requests focused and confined to their intended scope. Do not mix unrelated pre-existing issues (such as missing timeout handling) or their corresponding test updates into the current PR; address them in separate PRs instead.

Comment on lines +15 to +17
"source_files": [
"input_data/*.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.

medium

The source_files path should be updated to match the actual downloaded files directory (input_files/) instead of the obsolete input_data/ directory, as the new download script downloads and extracts files to input_files.

Suggested change
"source_files": [
"input_data/*.csv"
],
"source_files": [
"input_files/*.csv"
],

Comment on lines +15 to +17
"source_files": [
"input_files/ccd_enrollment_input.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.

medium

The source_files path should be updated to point to the raw downloaded file source_files/tabn203.65.xlsx instead of input_files/ccd_enrollment_input.csv to accurately reflect the download script's output folder and maintain consistency with other manifests.

Suggested change
"source_files": [
"input_files/ccd_enrollment_input.csv"
],
"source_files": [
"source_files/tabn203.65.xlsx"
],

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