Skip to content

adding download script, goldens and validation_config, updated mapping for latest data - #2136

Open
smarthg-gi wants to merge 2 commits into
datacommonsorg:masterfrom
smarthg-gi:ncses_employed_college_grad_fix
Open

adding download script, goldens and validation_config, updated mapping for latest data#2136
smarthg-gi wants to merge 2 commits into
datacommonsorg:masterfrom
smarthg-gi:ncses_employed_college_grad_fix

Conversation

@smarthg-gi

Copy link
Copy Markdown
Contributor

No description provided.

@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 introduces a dynamic data downloader and preprocessor (download.py) for the NCES employed college graduates dataset, replacing a hardcoded URL in manifest.json. It also adds golden data files and a validation configuration to verify the imported data, and simplifies the TMCF mapping. The reviewer recommended increasing the retry attempts to 10 when resolving the URL and downloading the file to ensure robust downloads from the external government website.

def resolve_url(landing_url=LANDING_PAGE_URL,
file_pattern=FILE_PATTERN,
headers=None,
tries=3,

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

According to the repository's general rules, when downloading data from unstable or unreliable external sources (such as this external government website), we should configure a higher number of retries (e.g., tries=10) to ensure download success. Please update the default tries parameter in resolve_url to 10.

Suggested change
tries=3,
tries=10,
References
  1. When downloading data from unstable or unreliable external sources, configure a higher number of retries (e.g., tries=10) to ensure download success, even if it results in a long cumulative wait time.

logging.error("Failed to resolve URL from landing page.")
sys.exit(1)

if not download_file(resolved_url, OUTPUT_FOLDER, False, None):

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

According to the repository's general rules, when downloading data from unstable or unreliable external sources, we should configure a higher number of retries (e.g., tries=10) to ensure download success. Please pass tries=10 to the download_file call.

Suggested change
if not download_file(resolved_url, OUTPUT_FOLDER, False, None):
if not download_file(resolved_url, OUTPUT_FOLDER, False, None, tries=10):
References
  1. When downloading data from unstable or unreliable external sources, configure a higher number of retries (e.g., tries=10) to ensure download success, even if it results in a long cumulative wait time.

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