-
Notifications
You must be signed in to change notification settings - Fork 220
feat(nimbus): add weekly metric view to popout card #14209
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
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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", | ||
| ), | ||
| } | ||
|
|
||
| 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" | ||
|
|
@@ -1560,7 +1582,6 @@ def get_metric_data( | |
| analysis_basis, | ||
| segment, | ||
| reference_branch, | ||
| window, | ||
| ) | ||
|
|
||
| return metric_data | ||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah like this clean small method |
||
|
|
||
| def get_sorted_branches(self): | ||
| return ( | ||
| [ | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@moibra05 is there a way we can break it down in smaller methods?
Uh oh!
There was an error while loading. Please reload this page.
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.
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