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

Flu inpatient #1476

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Flu inpatient #1476

wants to merge 10 commits into from

Conversation

bwilder0
Copy link

Description

Add a new signal for flu inpatient. This combines two new files from CHNG. The numerator is a count of new inpatient admissions with ICD code J09*-J11*. The denominator is the number of new inpatient admissions. Inpatient claims are identified by looking for both the presence of an admission date and the claim type + bill code matching an inpatient setting. The ratio is smoothed/adjusted in the same way as other CHNG signals.

Changelog

Itemize code/test/documentation changes and files added/removed.

  • load_data.py was changed to load the data for the new signal (the function load_flu_inpatient_data)
  • Small changes were made throughout run.py and update_sensor.py to accommodate the fact that this signal's base geography is the state level, not county. Several places had a hardcoded assertion that the base geography was 'fips'. This has been changed to permit 'state_code' when processing the flu inpatient signal.
  • Tests were updated both to pass in the base geography explicitly (as is now required) and to test load_flu_inpatient_data

@bwilder0 bwilder0 requested a review from krivard January 20, 2022 00:22
@bwilder0
Copy link
Author

Not sure why the linter is crashing at the very end...I'll take a look tomorrow but any suggestions are appreciated

@qx-teo
Copy link
Contributor

qx-teo commented Jan 20, 2022

Looks like the crash is due to something on a PR I submitted, lemme look into it!

@rumackaaron
Copy link
Contributor

@bwilder0 If you merge in the changes mentioned by @qx-teo do all of the checks pass?

@bwilder0
Copy link
Author

Hm looks like the same thing is still happening after merging the changes. @qx-teo, any thoughts on what might be happening here?

@qx-teo
Copy link
Contributor

qx-teo commented Jan 25, 2022

There was a failing test in _delphi_utils_python that is fixed by the merge, I'm not too sure what's causing the linter issue in changehc but that's probably not related to the issue I was talking about in my earlier comment.

Seems like the usual pylint complaints for the most part, although I've never seen the R1714 error.

@krivard
Copy link
Contributor

krivard commented Jan 27, 2022

@bwilder0 the commands you want are
make lint
and
make test

more information is in the default README.md file for each indicator.

Copy link
Contributor

@krivard krivard left a comment

Choose a reason for hiding this comment

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

👍

@krivard
Copy link
Contributor

krivard commented May 31, 2023

before merging, we should commit to writing API documentation for these new signals

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