-
Notifications
You must be signed in to change notification settings - Fork 17
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 test for NCHS national & proportional data #1942
Conversation
nchs_mortality/tests/test_run.py
Outdated
gmpr = GeoMapper() | ||
national_pop = gmpr.get_crosswalk("nation", "pop") | ||
us_pop = national_pop.loc[national_pop["nation"] == "us"]["pop"][0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just repeating the same logic added in #1938 ... I think its better to hardcode the US population value here in case GeoMapper starts behaving differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!
nchs_mortality/tests/test_run.py
Outdated
prop_value = pytest.approx(prop.iloc[0]["val"], 0.0001) | ||
INCIDENCE_BASE = 100000 | ||
assert(prop_value == num.iloc[0]["val"] / us_pop * INCIDENCE_BASE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this approx
comparison necessary?? These calculations should be deterministic since the test and reference data were generated on the same machine using virtually (if not exactly) the same operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well then, it might not be a bad idea to hardcode this too
@melange396 fixed the test, re-requesting review! I've also realized the test would technically pass if both national and proportional values were 0s, so added a check for that as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
Description
Adds an extra test for #1912 + #1938, making sure that national data is generated, and prop(ortional) signals are correctly calculated.