Sbachmei/mic 5668/bugfix handle empty years for means #387
Conversation
stevebachmeier
requested review from
albrja,
collijk,
hussain-jafari,
patricktnast and
rmudambi
as code owners
| data = data.filter(DEMOGRAPHIC_COLUMNS + data_type.value_columns) | ||
| data = utilities.normalize(data, data_type.value_columns, fill_value=0) |
There was a problem hiding this comment.
In multiple places throughout this PR you'll see I moved the filtering of columns prior to the normalizing. I do this b/c the normalizing includes various groupbys and filtering beforehand provides a very large speedup
| @@ -0,0 +1,252 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
I strongly urge you to ignore the first commit when reviewing (which does nothing more than copies this from the TESTS_TO_IMPLEMENT folder)
tests/unit/test_core.py
Outdated
| @@ -92,22 +88,18 @@ def check_year_in_data(entity, measure, location, years): | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("entity_details", ENTITIES_C, ids=lambda x: x[0].name) | |||
| @pytest.mark.parametrize("measure", MEASURES_C, ids=lambda x: x[0]) | |||
| @pytest.mark.parametrize("location", LOCATIONS_C) | |||
| def test_core_causelike(entity_details, measure, location): | |||
There was a problem hiding this comment.
throughout this module, I've deleted all of the test_core functions. That is already covered by test_get_measure.py.
tests/unit/test_core.py
Outdated
| entity, entity_expected_measures = entity_details | ||
| measure_name, measure_id = measure | ||
| tester = success_expected if (entity_expected_measures & measure_id) else fail_expected |
There was a problem hiding this comment.
I didn't like keeping success_expected and fail_expected around just for this one test so I implemented the raise vs return logic directly
stevebachmeier
deleted the
sbachmei/mic-5668/bugfix-handle-empty-years-for-means
branch
Bugfix handle different years from get_outputs
Description
Changes and notes
This implements a few of the (currently skipped) core.py integration tests
as (slow-running) unit tests (temporarily). In doing this, I found there is a bug
in how we are handling
years="all", data_type="means".Testing
All tests pass. NOTE: I'm skipping all relative_risk and paf parameters
since that takes SO long and a followup PR will use mocked data anyway.
i.e. I'm assuming there's nothing special about the location/year return
for rr and paf data.