Eatyourpeas/aggregation-logs-text-fixes #933
Conversation
…ed. Note problems still line 240
… values for abstraction_level
| # value_counts for this abstraction level already updated | ||
| # Set all measures to 0 for remaining abstraction levels | ||
| logger.debug(f"{len(list_of_updated_abstraction_level_instance)} scored {abstraction_level.name} instances updated with aggregated scores of a total {AbstractionKPIAggregationModel.objects.filter(cohort=cohort).count()} {abstraction_level.name}s") | ||
| empty_kpis = { |
There was a problem hiding this comment.
I think we can probably avoid writing the zero rows now we know that we must clear out all the aggregations rows and re-seed if we change the case filtering criteria at the top of the pipeline
| kpi_value_counts = ( | ||
| KPI.objects.filter( | ||
| registration__id__in=filtered_cases.values_list("registration") | ||
| registration__id__in=filtered_cases.values("registration") |
There was a problem hiding this comment.
Should this still be values_list as on line 266? I think if we change one we should change both
| return | ||
| ABSTRACTION_CODE = value_count.pop(f"organisation__{abstraction_level.value}") #value_count.get(next(iter(value_count))) # | ||
| if ABSTRACTION_CODE is not None: | ||
| # the last value in each abstraction_level is None - discard |
There was a problem hiding this comment.
I agree with removing the early return but maybe the comment is clearer above the if?
I also don't think the last value is None, I think it's if we're grouping by an abstraction level not relevant here eg by local health board in England
| cohort=6 | ||
| ) | ||
| ) | ||
| # kpi_aggregation_model_instance = ( |
|
@eatyourpeas what's left to get this merged? :) |
There was a problem hiding this comment.
I am not sure how much extra this PR really adds any more. The new filter function is already in live, so this was really about the logs, the zero rows and maybe a test update.
Perhaps this is one we can do together or we could just close if it nolonger adds value?
Overview
Another incremental improvement in the aggregation suite.
This ensures logs for any updates or creates to the KPIAggregation models, with a summary of how many models changed, and how many records updated with 0 if unscored.
There are also some fixes to the tests:
filter_completed_cases_at_one_year_by_abstraction_levelso testing closer to real lifeCode changes
filter_completed_cases_at_one_year_by_abstraction_levelto__init__.pyto allow easier importingreturnstatement after testing for None, and nest any code below this test into a conditional._register_kpi_scored_casesfunction in the e12case factory to include cases who have completed a full year of careget_filtered_cases_queryset_forwhich was now failing update KPIAggregation table tests, and replace withfilter_completed_cases_at_one_year_by_abstraction_levelwhich is used in the actual calculations anywayDocumentation changes (done or required as a result of this PR)
This needs doing - mostly to make explicit that the aggregation tables now only contain fully scored cases that have completed a full year of care
Related Issues
Closes #907