Ejscreen semi-automatic #1184
Ejscreen semi-automatic #1184
Conversation
scripts/us_epa/ejscreen/ejscreen.py
Outdated
| with zfile.open(f'{FILENAMES[year]}.csv', 'r') as newfile: | ||
| dfs[year] = pd.read_csv(newfile, usecols=columns) | ||
| # some years are not zipped | ||
| if year == '2024': |
There was a problem hiding this comment.
'year' value should be taken from the config file, it should not be hard coded.
scripts/us_epa/ejscreen/ejscreen.py
Outdated
| dfs[year] = pd.read_csv(newfile, usecols=columns) | ||
| # some years are not zipped | ||
| if year == '2024': | ||
| url = f'https://gaftp.epa.gov/EJSCREEN/2024/2.32_August_UseMe/{zip_filename}.zip' |
There was a problem hiding this comment.
'url' value also move to config file and try to access from there.
scripts/us_epa/ejscreen/config.json
Outdated
There was a problem hiding this comment.
Please remove this file from git and update it in GCS location.
scripts/us_epa/ejscreen/ejscreen.py
Outdated
| _CONFIG_PATH = os.path.join(_MODULE_DIR, 'config.json') | ||
|
|
||
| # Load configuration from config.json | ||
| with open(_CONFIG_PATH, 'r') as f: |
There was a problem hiding this comment.
config_path from the flag is not considered here. Config file is reading from _MODULE_DIR location. Can you modify this to read the config file from GCS location?
There was a problem hiding this comment.
Please add a flag for download and process separately .
There was a problem hiding this comment.
I couldn't find the flag for download and process separately. Can you check if the updated code is checked-in properly?
scripts/us_epa/ejscreen/ejscreen.py
Outdated
| ) | ||
|
|
||
| # Rename columns to match other years | ||
| if year == '2024': |
There was a problem hiding this comment.
Avoid hard coding year like '2024' instead add a list/dict in config file for column remaining and check like below
if year in _CONFIG['remane_colums']:
cols_renamed = dict(zip(columns, NORM_CSV_COLUMNS1))
else:
cols_renamed = dict(zip(columns, NORM_CSV_COLUMNS))
scripts/us_epa/ejscreen/ejscreen.py
Outdated
| logger.info( | ||
| f"File downloaded and processed for {year} successfully") | ||
| else: | ||
| logger.error( |
There was a problem hiding this comment.
Please change logging.error to logging.fatal
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| def main(_): |
There was a problem hiding this comment.
Add try/catch block in the main method to avoid partial import in case of unexpected errors.
| "cleaned_csv": "ejscreen_airpollutants.csv" | ||
| } | ||
| ], | ||
| "cron_schedule": "0 07 * * 1" |
There was a problem hiding this comment.
No need to prefix '0' in hour field. Please make it as "0 7 * * 1"
|
|
||
|
|
||
| # Download the file and save it in the input folder | ||
| def download_file(url, year, zip_filename=None): |
There was a problem hiding this comment.
Can you please implement the retry module for this get request?
refer to this like https://pypi.org/project/retry/
|
|
||
| except Exception as e: | ||
| logger.fatal(f"Unexpected error in the main process: {e}") | ||
| sys.exit(1) |
There was a problem hiding this comment.
sys.exit(1) is not required here, You can remove this.
| "import_specifications": [ | ||
| { | ||
| "import_name": "EPA_EJSCREEN", | ||
| "curator_emails": ["[email protected]"], |
There was a problem hiding this comment.
Please remove email id from curator_emails and keep it like "curator_emails": [],
No description provided.