Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add management command to patch ahhs data. #632

Merged
merged 2 commits into from
Jun 5, 2024
Merged

Conversation

Rup-Narayan-Rajbanshi
Copy link
Contributor

@Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi commented Jun 3, 2024

Addresses

Changes

  • patch figure data whose household size were updated manually.

This PR doesn't introduce any:

  • temporary files, auto-generated files or secret keys
  • n+1 queries
  • flake8 issues
  • print
  • typos
  • unwanted comments

This PR contains valid:

  • tests
  • permission checks (tests here too)
  • translations

CSV File

manually_updated_ahhs_figures_mar_13.csv

Copy link
Member

@thenav56 thenav56 left a comment

Choose a reason for hiding this comment

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

Looks good.

apps/entry/management/commands/fix_ahhs_data.py Outdated Show resolved Hide resolved
apps/entry/management/commands/fix_ahhs_data.py Outdated Show resolved Hide resolved
apps/entry/management/commands/fix_ahhs_data.py Outdated Show resolved Hide resolved
apps/entry/management/commands/fix_ahhs_data.py Outdated Show resolved Hide resolved
apps/entry/management/commands/fix_ahhs_data.py Outdated Show resolved Hide resolved
@Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi force-pushed the feature/fix-ahhs-data branch 2 times, most recently from c99f349 to 7b0f4df Compare June 4, 2024 09:16

class Command(BaseCommand):

help = "Fix AHHS household size data."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
help = "Fix AHHS household size data."
help = "Patch AHHS household size data."

Comment on lines 74 to 78
logger.info("2024 AHHS data fixed successfully.")
except FileNotFoundError:
logger.error(f"CSV file at {csv_file_path} not found.")
except Exception:
logger.error("Failed to fix 2024 AHHS data:", exc_info=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger.info("2024 AHHS data fixed successfully.")
except FileNotFoundError:
logger.error(f"CSV file at {csv_file_path} not found.")
except Exception:
logger.error("Failed to fix 2024 AHHS data:", exc_info=True)
logger.info("AHHS data patched successfully.")
except FileNotFoundError:
logger.error(f"CSV file at {csv_file_path} not found.")
except Exception:
logger.error("Failed to patch AHHS data:", exc_info=True)

Comment on lines 59 to 62
self.stdout.write(f'Figure id not found ids: {figure_id_not_found}')
self.stdout.write(f'Figure unit not household ids: {change_in_unit_figures}')
self.stdout.write(f'Change in reported value figure ids: {change_in_reported_value_figures}')
self.stdout.write(f'No change in household size figure ids: {no_change_in_household_figures}')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we log these as error?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, shouldn't we use logger?

self.stdout.write(f'Change in reported value figure ids: {change_in_reported_value_figures}')
self.stdout.write(f'No change in household size figure ids: {no_change_in_household_figures}')
self.stdout.write(f'Total number of figures patched: {len(patched_figure_ids)}')
self.stdout.write(f'Figure Ids patched with old household and total figures: {patched_figure_ids}')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.stdout.write(f'Figure Ids patched with old household and total figures: {patched_figure_ids}')
self.stdout.write(f'IDs of figures patched: {patched_figure_ids}')

apps/entry/management/commands/fix_ahhs_data.py Outdated Show resolved Hide resolved
apps/entry/management/commands/fix_ahhs_data.py Outdated Show resolved Hide resolved
Comment on lines 77 to 76
self.stdout.write(f"Total number of figures patched: {len(patched_figure_ids)}")
self.stdout.write(f"IDs of figures patched: {patched_figure_ids}")
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not have any information.

@tnagorra tnagorra force-pushed the feature/fix-ahhs-data branch 2 times, most recently from cd88c9c to 1459fb2 Compare June 5, 2024 03:54
thenav56
thenav56 previously approved these changes Jun 5, 2024
- Additionally log bulk update summary
@tnagorra tnagorra merged commit 2449799 into develop Jun 5, 2024
1 of 2 checks passed
@tnagorra tnagorra deleted the feature/fix-ahhs-data branch June 5, 2024 04:05
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.

3 participants