Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 124 additions & 61 deletions experimenter/experimenter/experiments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from collections import defaultdict
from dataclasses import dataclass
from decimal import Decimal
from itertools import chain
from itertools import chain, zip_longest
from pathlib import Path
from typing import Any, Optional
from urllib.parse import urlencode, urljoin
Expand Down Expand Up @@ -1437,78 +1437,100 @@ def get_branch_data(self, analysis_basis, selected_segment, window="overall"):

return branch_data

def get_metric_area_data(
self, metrics, analysis_basis, segment, reference_branch, window="overall"
):
metric_data = {}
metric_area_data = {"metrics": metrics, "data": metric_data}
def get_metric_area_data(self, metrics, analysis_basis, segment, reference_branch):
def get_window_metric_data(reference_branch, window_results, window):
window_metric_data = {}

window_results = self.get_window_results(analysis_basis, segment, window)

for metric in metrics:
slug = metric.get("slug")
group = metric.get("group")
branch_metrics = {}
for metric in metrics:
slug = metric.get("slug")
group = metric.get("group")

for branch in self.get_sorted_branches():
branch_results = window_results.get(branch.slug, {}).get(
"branch_data", {}
branch_metrics = self.build_branch_metrics(
group, slug, window_results, reference_branch, window
)

metric_src = branch_results.get(group, {}).get(slug, {})
absolute_data_list = metric_src.get("absolute", {}).get("all", [])
relative_data_list = (
metric_src.get("relative_uplift", {})
.get(reference_branch, {})
.get("all", [])
)
window_metric_data[slug] = branch_metrics

def formatted_analysis_point_comparator(point):
wi = point.get("window_index")
return int(wi) if wi is not None else 0
return window_metric_data

absolute_data_list.sort(key=formatted_analysis_point_comparator)
relative_data_list.sort(key=formatted_analysis_point_comparator)
metric_data = {
"overall": get_window_metric_data(
reference_branch,
self.get_window_results(analysis_basis, segment, "overall"),
"overall",
),
"weekly": get_window_metric_data(
reference_branch,
self.get_window_results(analysis_basis, segment, "weekly"),
"weekly",
Comment on lines 1452 to 1465
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moibra05 is there a way we can break it down in smaller methods?

Copy link
Contributor Author

@moibra05 moibra05 Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I broke up the method into several smaller ones and I intend to eventually move them all into their own file. created an issue for this #14212

),
}

significance_map = (
metric_src.get("significance", {})
.get(reference_branch, {})
.get(window, {})
)
metric_area_data = {"metrics": metrics, "data": metric_data}

abs_entries = []
for i, data_point in enumerate(absolute_data_list):
lower = data_point.get("lower")
upper = data_point.get("upper")
return metric_area_data

significance = significance_map.get(str(i + 1), "neutral")
abs_entries.append(
{"lower": lower, "upper": upper, "significance": significance}
)
def window_index_for_sort(self, point):
wi = point.get("window_index")
return int(wi) if wi is not None else 0

def format_absolute_entries(self, metric_src, significance_map):
absolute_data_list = metric_src.get("absolute", {}).get("all", [])
absolute_data_list.sort(key=self.window_index_for_sort)
abs_entries = []
for i, data_point in enumerate(absolute_data_list):
lower = data_point.get("lower")
upper = data_point.get("upper")
significance = significance_map.get(str(i + 1), "neutral")
abs_entries.append(
{"lower": lower, "upper": upper, "significance": significance}
)
return abs_entries

def format_relative_entries(self, metric_src, significance_map, reference_branch):
relative_data_list = (
metric_src.get("relative_uplift", {}).get(reference_branch, {}).get("all", [])
)
relative_data_list.sort(key=self.window_index_for_sort)
rel_entries = []
for i, data_point in enumerate(relative_data_list):
lower = data_point.get("lower")
upper = data_point.get("upper")
avg_rel_change = abs(data_point.get("point"))
significance = significance_map.get(str(i + 1), "neutral")
rel_entries.append(
{
"lower": lower,
"upper": upper,
"significance": significance,
"avg_rel_change": avg_rel_change,
}
)
return rel_entries

rel_entries = []
for i, data_point in enumerate(relative_data_list):
lower = data_point.get("lower")
upper = data_point.get("upper")
avg_rel_change = abs(data_point.get("point"))
significance = significance_map.get(str(i + 1), "neutral")
rel_entries.append(
{
"lower": lower,
"upper": upper,
"significance": significance,
"avg_rel_change": avg_rel_change,
}
)
def build_branch_metrics(self, group, slug, window_results, reference_branch, window):
branch_metrics = {}
for branch in self.get_sorted_branches():
branch_results = window_results.get(branch.slug, {}).get("branch_data", {})
metric_src = branch_results.get(group, {}).get(slug, {})

branch_metrics[branch.slug] = {
"absolute": abs_entries,
"relative": rel_entries,
}
significance_map = (
metric_src.get("significance", {})
.get(reference_branch, {})
.get(window, {})
)

metric_data[slug] = branch_metrics
abs_entries = self.format_absolute_entries(metric_src, significance_map)
rel_entries = self.format_relative_entries(
metric_src, significance_map, reference_branch
)

return metric_area_data
branch_metrics[branch.slug] = {
"absolute": abs_entries,
"relative": rel_entries,
}

return branch_metrics

def get_kpi_metrics(
self, analysis_basis, segment, reference_branch, window="overall"
Expand Down Expand Up @@ -1560,7 +1582,6 @@ def get_metric_data(
analysis_basis,
segment,
reference_branch,
window,
)

return metric_data
Expand Down Expand Up @@ -1606,6 +1627,48 @@ def get_max_metric_value(

return max_value

def get_weekly_metric_data(self, analysis_basis, segment, reference_branch):
all_metrics = self.get_metric_data(analysis_basis, segment, reference_branch)

weekly_metric_data = {}

for metric_data in all_metrics.values():
metadata = metric_data.get("metrics", {})

for metric_metadata in metadata:
data = (
metric_data.get("data", {})
.get("weekly", {})
.get(metric_metadata["slug"], {})
)
weekly_data = {}

for branch_slug, branch_data in data.items():
# Always produce a list of pairs by zipping absolute and relative
# When one side is missing or shorter, pad it with None so templates
# can iterate without complex conditionals.
abs_list = branch_data.get("absolute") or []
rel_list = branch_data.get("relative") or []

weekly_data[branch_slug] = list(
zip_longest(abs_list, rel_list, fillvalue=None)
)

weekly_metric_data[metric_metadata["slug"]] = weekly_data

return weekly_metric_data

def get_weekly_dates(self):
weekly_dates = []
if not self.is_rollout:
if self._enrollment_end_date:
date = self._enrollment_end_date
while (date + datetime.timedelta(days=7)) <= self.computed_end_date:
weekly_dates.append((date, date + datetime.timedelta(days=7)))
date += datetime.timedelta(days=7)

return weekly_dates
Comment on lines +1661 to +1670
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah like this clean small method


def get_sorted_branches(self):
return (
[
Expand Down
33 changes: 31 additions & 2 deletions experimenter/experimenter/experiments/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1418,6 +1418,35 @@ def test_targeting_config_returns_None_with_invalid_slug(self):
experiment = NimbusExperimentFactory.create(targeting_config_slug="invalid slug")
self.assertIsNone(experiment.targeting_config)

@parameterized.expand(
[
[
datetime.date(2025, 1, 1),
datetime.date(2025, 1, 30),
[
(datetime.date(2025, 1, 1), datetime.date(2025, 1, 8)),
(datetime.date(2025, 1, 8), datetime.date(2025, 1, 15)),
(datetime.date(2025, 1, 15), datetime.date(2025, 1, 22)),
(datetime.date(2025, 1, 22), datetime.date(2025, 1, 29)),
],
],
[
datetime.date(2025, 1, 1),
datetime.date(2025, 1, 8),
[(datetime.date(2025, 1, 1), datetime.date(2025, 1, 8))],
],
[datetime.date(2025, 1, 1), datetime.date(2025, 1, 3), []],
]
)
def test_get_weekly_dates_for_experiment(self, start_date, end_date, expected_dates):
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.ENDING_APPROVE_APPROVE,
_enrollment_end_date=start_date,
_computed_end_date=end_date,
)

self.assertEqual(experiment.get_weekly_dates(), expected_dates)

def test_start_date_returns_None_for_not_started_experiment(self):
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.CREATED,
Expand Down Expand Up @@ -2487,8 +2516,8 @@ def test_get_metric_data_returns_correct_data(self):
results_a = experiment.get_metric_data("enrollments", "all", "branch-a")
results_b = experiment.get_metric_data("enrollments", "all", "branch-b")

data_a = results_b.get("KPI Metrics", {}).get("data", {})
data_b = results_a.get("KPI Metrics", {}).get("data", {})
data_a = results_b.get("KPI Metrics", {}).get("data", {}).get("overall", {})
data_b = results_a.get("KPI Metrics", {}).get("data", {}).get("overall", {})

self.assertEqual(
data_a.get("retained").get(branch_a.slug).get("absolute"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,27 @@

{% with sig=significance|default:"neutral" %}
{% if slug == reference_branch %}
<div class="card p-4 bg-body-tertiary text-muted border-0 text-center d-flex flex-row align-items-center justify-content-center"
<div class="card p-4 bg-body-tertiary text-muted border-0 text-center d-flex align-items-center justify-content-center"
style="height: 100px">
{% if absolute_lower %}
{% if display_type == "percentage" %}
({{ absolute_lower|to_percentage:1 }} to {{ absolute_upper|to_percentage:1 }})
<span class="mb-0 me-1"
data-bs-toggle="tooltip"
data-bs-title="{{ absolute_lower|to_percentage }}">({{ absolute_lower|to_percentage:1 }}</span>
to
<span class="mb-0 ms-1"
data-bs-toggle="tooltip"
data-bs-title="{{ absolute_upper|to_percentage }}">{{ absolute_upper|to_percentage:1 }})</span>
{% else %}
({{ absolute_lower|floatformat:2 }} to {{ absolute_upper|floatformat:2 }})
<span class="mb-0 me-1"
data-bs-toggle="tooltip"
data-bs-title="{{ absolute_lower }}">({{ absolute_lower|floatformat:2 }}</span>
to
<p class="mb-0 ms-1"
data-bs-toggle="tooltip"
data-bs-title="{{ absolute_upper }}">
{{ absolute_upper|floatformat:2 }})
</span>
{% endif %}
{% else %}
Baseline
Expand All @@ -20,18 +34,36 @@
<i class="fa-solid {% if sig == "positive" %}fa-arrow-up{% elif sig == "negative" %}fa-arrow-down{% else %}fa-arrow-left{% endif %}"></i>
<div class="d-flex flex-column align-items-center">
{% if sig == "positive" %}
<span class="fw-semibold">{{ percentage|to_percentage:1 }} increase</span>
<span class="fw-semibold"
data-bs-toggle="tooltip"
data-bs-title="{{ percentage|to_percentage }}">{{ percentage|to_percentage:1 }} increase</span>
{% elif sig == "negative" %}
<span class="fw-semibold">{{ percentage|to_percentage:1 }} decrease</span>
<span class="fw-semibold"
data-bs-toggle="tooltip"
data-bs-title="{{ percentage|to_percentage }}">{{ percentage|to_percentage:1 }} decrease</span>
{% else %}
<span class="fw-semibold">No notable change</span>
{% endif %}
<span class="mb-0">
<span class="mb-0 d-flex">
{% if absolute_lower %}
{% if display_type == "percentage" %}
({{ absolute_lower|to_percentage:1 }} to {{ absolute_upper|to_percentage:1 }})
<span class="mb-0 me-1"
data-bs-toggle="tooltip"
data-bs-title="{{ absolute_lower|to_percentage }}">({{ absolute_lower|to_percentage:1 }}</span>
to
<span class="mb-0 ms-1"
data-bs-toggle="tooltip"
data-bs-title="{{ absolute_upper|to_percentage }}">{{ absolute_upper|to_percentage:1 }})</span>
{% else %}
({{ absolute_lower|floatformat:2 }} to {{ absolute_upper|floatformat:2 }})
<span class="mb-0 me-1"
data-bs-toggle="tooltip"
data-bs-title="{{ absolute_lower }}">({{ absolute_lower|floatformat:2 }}</span>
to
<p class="mb-0 ms-1"
data-bs-toggle="tooltip"
data-bs-title="{{ absolute_upper }}">
{{ absolute_upper|floatformat:2 }})
</span>
{% endif %}
{% endif %}
</span>
Expand Down
Loading