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.

Sign up for GitHub

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

Jump to bottom

Switch to new flusurv API endpoint #1278

Open
wants to merge 43 commits into
base: dev
Choose a base branch
from
Open

Switch to new flusurv API endpoint #1278

wants to merge 43 commits into from

Conversation

nmdefries
Copy link
Contributor

@nmdefries nmdefries commented Aug 23, 2023 •
edited

Summary:

Pull flusurv data from new CDC API endpoint. Ingest previously unlabelled age groupings. Ingest new race/ethnicity and sex breakdowns. Test flusurv.py functions. Add more comments, messaging, and assertions.

Closes #1247
Closes #242

Note:

  • Some of our CDC contacts are able to provide us with past versions of data from the API. We will want to patch those in. Many of the "new" strata are actually available back to 2009. We'll probably want to patch those in as well.
  • This also requires updates to the database table, the API server, and the API docs that will be made separately.

Prerequisites:

  • Unless it is a documentation hotfix it should be merged against the dev branch
  • Branch is up-to-date with the branch to be merged with, i.e. dev
  • Build is successful
  • Code is cleaned up and formatted

@nmdefries
initial switch to new API endpoint; doesn't account for format change
d99d2d0
@nmdefries
separate fetch fn for whole api obj
70c3e16
@nmdefries
get_current_issue to use existing json response
3580f3f
@nmdefries
cleanup names and comments
9cb1da4 
- rename input arg to `update` to avoid reassignment later
- comment and reuse args_insert
- spelling
- comment magic constant used in output format
- rename location-network/catchmentid map
@nmdefries
store extracted rates by strata name rather than position
9bf3c82 
Previously, age strata were numbered sequentially which allowed us to
store rate values by position in a list. With the introduction of the
new strata, this system is not robust enough to track all the different
groups (e.g. ageids are no longer sequential and there are now race and
sex groupings with separate numbering systems).
@nmdefries
define function to convert json obs to dict grouped by location and e…
cea25ab 
…piweek
@nmdefries
auto-map from valueids to ordinal and label-based group names
24dc088
@nmdefries
add new strata to sql insert statement by name, not order
26eff97
@nmdefries
pass seasonids around to use in requests for location-specific data
229a96c
@nmdefries
include old and new example API responses
1e942a5
@nmdefries
flusurv tests
f8a6706
@nmdefries nmdefries marked this pull request as ready for review September 15, 2023 16:25
@nmdefries nmdefries removed the request for review from brookslogan September 15, 2023 21:51
@nmdefries nmdefries mentioned this pull request Sep 15, 2023
Merged
4 tasks
This was referenced Sep 18, 2023
Open
Open
melange396
melange396 requested changes Sep 26, 2023
View reviewed changes
Copy link
Collaborator

@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.

nice work! i really appreciate the thorough commenting!

youll wanna pull in the changes from the dev branch, PR #1241 added tests for the flusurv endpoint.

src/acquisition/flusurv/flusurv.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv_update.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv_update.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv_update.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv_update.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv_update.py Outdated Show resolved Hide resolved
@nmdefries
convert asserts to if-raise
c7829ee
@nmdefries
for robust group checking, list all expected groups and do set diff
c91b7e8
@nmdefries
separate data fetching fns into metadata and location classes
03926dd
@nmdefries
programmatically create location-networkid map; create constants file
c5d8d4e
@nmdefries
switch missing data assert to exception
9ed92d8
@nmdefries
Merge branch 'dev' into ndefries/flusurv-new-endpoint
1c9e730
@nmdefries
mock metadata and api objs for tests
eb920cb
@nmdefries
stop doing recent season filtering
b191602 
Catchment metadata is not updated with most recent season, so doing this
will stop us from fetching data.
@nmdefries
warn when no location data returned
b7ba101
@nmdefries
test metadata attributes together
e1d9d06
@nmdefries
correct local module name to load classes from
772e127
@nmdefries
map some long race groups to abbreviated names
991fde1
@nmdefries
programmatically create sql query to avoid ordering issues
5d00dbb
@nmdefries
return empty result with right format when no data available
5cedb30
@nmdefries
warning prefix
e7ad06e
@nmdefries
add tests for geo-abbr conversion and location fetching
75283d7
@nmdefries
don't error for missing groups -- some are expected
276944a
@nmdefries
Merge branch 'dev' into ndefries/flusurv-new-endpoint
6c531a6
@nmdefries
fn to add empty sex breakdowns to UT
9431489
@nmdefries
with UT sex subsets getting filled, error on missing groups again
205c061
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 9 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

This was referenced Nov 8, 2023
Open
Open
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Dec 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 9 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

nmdefries and others added 2 commits May 7, 2024 20:15
@nmdefries
Update flusurv schema and docs with new age, sex, and race groups (#1287
9de7d64 
)

* add new columns to flusurv endpoint spec

* add new age, sex, race strata, and season to schema

* list new signals in documentation

* move new season column to end of table

* add migration script to add new fields to flusurv table

* add field descriptions; reformat table so don't need to match line lengths

* point from acquisition script to docs and schema def to deduplicate
@melange396
Revert "Update flusurv schema and docs with new age, sex, and race gr…
40212bb 
…oups" (#1426)

This reverts commit 9de7d64.
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 8, 2024

Quality Gate Passed Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@melange396
Copy link
Collaborator

I wasnt paying attention and accidentally merged #1287 into this PR. I then reverted #1287 in #1426. This is a reminder that #1287 will eventually need to be re-merged into here (which is now probably best done by reverting the revert in #1426). Sorry for the confusion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brookslogan brookslogan brookslogan left review comments

@melange396 melange396 Awaiting requested review from melange396

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

flusurv data is stale flusurv acquisition is broken
3 participants
@nmdefries @melange396 @brookslogan