Skip to content

Conversation

@dsweber2
Copy link
Contributor

@dsweber2 dsweber2 commented Dec 7, 2023

Adding state and national aggregations of the NWSS wastewater data. Aggregation from the sample site to state/nation is weighted by population (with NA population values not included for the total for their respective states; we don't have an exhaustive sample anyway), with the population estimates taken from the corresponding "metric" dataset on a day by day basis. I couldn't use the geo-mapper because the population_served depends on the day, sometimes by an order of magnitude.

For previous discussion, see this issue. That will be ongoing after this, as there's still sample-site level and county-level aggregations to include. Those are more involved.

make lint doesn't seem to be working on my machine, even if the actual commands run on their own do. Not sure why we're pinning the python version to 3.8 to run that. It's at 8.9/10 when run via pylint delphi_NWSS directly, and the problems are things like unable to import delphi_tools.

Currently still needs tests; env/bin/python -m delpi_NWSS works, with the output confirmed to be of the right general form.

@rlunde I'm not sure who exactly is the right person to be tagging this for review

@dsweber2 dsweber2 self-assigned this Dec 7, 2023
@nmdefries nmdefries requested review from a team and nolangormley and removed request for a team December 11, 2023 22:39
Copy link
Contributor

@nolangormley nolangormley left a comment

Choose a reason for hiding this comment

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

Mostly looks good.

Missing a receiving folder, tests, and some small nitpicks.

Copy link
Contributor

@nolangormley nolangormley left a comment

Choose a reason for hiding this comment

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

LGTM!

@dsweber2 dsweber2 merged commit 833e818 into main Jan 8, 2024
@dshemetov dshemetov deleted the nwss branch January 8, 2024 19:59
Copy link
Contributor

@melange396 melange396 left a comment

Choose a reason for hiding this comment

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

sorry for the late review. any indication on how long this takes to run (average/typical)?

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.

5 participants