Skip to content

Commit

Permalink
[Aggregated Metrics] Refactor generate_aggregation_query_fragment() f…
Browse files Browse the repository at this point in the history
…unctions to support new optimized query format (Recidiviz/recidiviz-data#34812)

## Description of the change

Updates the `generate_aggregation_query_fragment()` functions to take in
`filter_observations_by_type` and
`read_observation_attributes_from_json` arguments and pass those values
through to `get_observation_conditions_string*()` functions.

This will allow us to reuse these aggregation logic fragments when
building aggregated metrics queries that referenced the new, smaller
unpacked observation queries introduced in Recidiviz/recidiviz-data#34725.

I checked and there were no changes to aggregated metrics queries
introduced by this change.

## Type of change

> All pull requests must have at least one of the following labels
applied (otherwise the PR will fail):

| Label | Description |
|-----------------------------
|-----------------------------------------------------------------------------------------------------------
|
| Type: Bug | non-breaking change that fixes an issue |
| Type: Feature | non-breaking change that adds functionality |
| Type: Breaking Change | fix or feature that would cause existing
functionality to not work as expected |
| Type: Non-breaking refactor | change addresses some tech debt item or
prepares for a later change, but does not change functionality |
| Type: Configuration Change | adjusts configuration to achieve some end
related to functionality, development, performance, or security |
| Type: Dependency Upgrade | upgrades a project dependency - these
changes are not included in release notes |

## Related issues

Part of Recidiviz/recidiviz-data#29291

## Checklists

### Development

**This box MUST be checked by the submitter prior to merging**:
- [x] **Double- and triple-checked that there is no Personally
Identifiable Information (PII) being mistakenly added in this pull
request**

These boxes should be checked by the submitter prior to merging:
- [ ] Tests have been written to cover the code changed/added as part of
this pull request

### Code review

These boxes should be checked by reviewers prior to merging:

- [x] This pull request has a descriptive title and information useful
to a reviewer
- [x] Potential security implications or infrastructural changes have
been considered, if relevant

GitOrigin-RevId: bc87d23a0e0a801f21563c79c44914321fe523bc
  • Loading branch information
ageiduschek authored and Helper Bot committed Nov 26, 2024
1 parent 43815fa commit 1255ba2
Show file tree
Hide file tree
Showing 11 changed files with 391 additions and 81 deletions.
2 changes: 1 addition & 1 deletion recidiviz/aggregated_metrics/aggregated_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def generate_aggregated_metrics_view_builder(
metrics_query_str = ",\n ".join([metric.name for metric in included_metrics])
view_description_metrics = [
f"|{metric.display_name} (`{metric.name}`)|{metric.description}|{metric.pretty_name()}|"
f"`{metric.get_observation_conditions_string_no_newline() if isinstance(metric, MetricConditionsMixin) else 'N/A'}`|"
f"`{metric.get_observation_conditions_string_no_newline(filter_by_observation_type=True, read_observation_attributes_from_json=True) if isinstance(metric, MetricConditionsMixin) else 'N/A'}`|"
for metric in included_metrics
]
view_description_metrics_str = "\n".join(view_description_metrics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
)


# TODO(#29291): This function should become unused once we've migrated over to optimized
# aggregated metrics queries.
def get_assignment_event_time_specific_cte(
unit_of_analysis: MetricUnitOfAnalysis,
population_type: MetricPopulationType,
Expand Down Expand Up @@ -86,6 +88,8 @@ def get_assignment_event_time_specific_cte(
metric_aggregation_fragment_inner = ",\n".join(
[
metric.generate_aggregation_query_fragment(
filter_observations_by_type=True,
read_observation_attributes_from_json=True,
event_date_col="events.event_date",
assignment_date_col="assign.assignment_date",
)
Expand Down Expand Up @@ -151,6 +155,8 @@ def get_assignment_event_time_specific_cte(
)


# TODO(#29291): This function should become unused once we've migrated over to optimized
# aggregated metrics queries.
def generate_assignment_event_aggregated_metrics_view_builder(
unit_of_analysis: MetricUnitOfAnalysis,
population_type: MetricPopulationType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
from recidiviz.observations.span_type import SpanType


# TODO(#29291): This function should become unused once we've migrated over to optimized
# aggregated metrics queries.
def get_assignment_span_time_specific_cte(
unit_of_analysis: MetricUnitOfAnalysis,
population_type: MetricPopulationType,
Expand Down Expand Up @@ -85,6 +87,8 @@ def get_assignment_span_time_specific_cte(
metric_aggregation_fragment_inner = ",\n".join(
[
metric.generate_aggregation_query_fragment(
filter_observations_by_type=True,
read_observation_attributes_from_json=True,
span_start_date_col="spans.start_date",
span_end_date_col="spans.end_date",
assignment_date_col="assign.assignment_date",
Expand Down Expand Up @@ -155,6 +159,8 @@ def get_assignment_span_time_specific_cte(
)


# TODO(#29291): This function should become unused once we've migrated over to optimized
# aggregated metrics queries.
def generate_assignment_span_aggregated_metrics_view_builder(
unit_of_analysis: MetricUnitOfAnalysis,
population_type: MetricPopulationType,
Expand Down
Loading

0 comments on commit 1255ba2

Please sign in to comment.