CHC Signal #320
CHC Signal #320
Conversation
|
Prefer using |
The code actually does use |
|
|
||
|
|
||
| class Constants: | ||
| # number of counties in usa, including megacounties |
There was a problem hiding this comment.
can we add a docstring here on what these constants are for, especially since there's also a constants.py file?
There was a problem hiding this comment.
this + linter and should be good to go. EDIT: there's also a few more instances of single vs double quoting, should be a relatively quick search and replace
|
I'm getting some keyerrors when running tests and If I update the params json to point to the two files in Getting a number of linting errors, mostly minor. |
|
Please drop the following files:
|
changehc/delphi_changehc/run.py
Outdated
| denom_suffix = params["input_denom_file"].split("/")[-1].split(".")[0][9:] | ||
| assert denom_suffix == "All_Outpatients_By_County" |
There was a problem hiding this comment.
might be simpler to regex here but this works. can we add tests for this file?
There was a problem hiding this comment.
Please drop the following files:
static/02_20_uszips.csvtests/static/02_20_uszips.csvparams.json(sorry about that; i completely spaced that both the code reviewer and the code writer were working in a relatively unfamiliar codebase)
Thanks Katie. There's a tests/params.json I think is ok to be commited (and needs to be set specifically for tests to pass)
There was a problem hiding this comment.
Realized i quoted the wrong comment here
| # Denominator: "YYYYMMDD_All_Outpatients_By_County.dat.gz" | ||
| # Numerator: "YYYYMMDD_Covid_Outpatients_By_County.dat.gz" | ||
|
|
||
| if params["drop_date"] is None: |
There was a problem hiding this comment.
is it worth testing some of this functionality? just to make sure that all the right files/dates get generated
There was a problem hiding this comment.
The params are loaded straight from params.json so I don't think this can be tested very easily. There is similar code for the hosp signal and this functionality wasn't tested.
|
Aside from open comments, looks good and tests are passing |
This creates a signal from the CHC outpatient data, using the same calculations as the EMR signal. The code currently works for CHC data, but I haven't written code that automates data collection from the ftp server.