Statistics #36
Statistics #36
Conversation
|
This is what the data looks like in BigQuery when running the integration test: |
There was a problem hiding this comment.
I don't think I really understand the apply/transform methods; don't we just need one of them?
Pretreatments and statistics do different things so maybe it makes sense for the methods to be named different things -- pretreatments transform dataframes to dataframes, and statistics transform dataframes to StatisticsResultCollections.
|
It looks like we're hitting python/mypy#4717. We should just |
|
I made changes to represent statistics in the config for metrics like discussed: [metrics]
weekly = ["active_ticks"]
[metrics.active_ticks.statistics.bootstrap_quantiles]
quantiles = [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0]
[metrics.active_ticks.statistics.statistic_without_arguments]
[metrics.active_ticks.statistics.bootstrap_means]
pre_treatments = ["trim", "log"]
[metrics.my_cool_metric]
select_expr = "1"
data_source = "whatever"
# This "inline table" syntax produces the same output as the explicit table syntax
statistics = {statistic_without_arguments={}, bootstrap_quantiles={}}I'll add more tests if the current approach looks okay. |
| Summary( | ||
| metric=mozanalysis.metrics.desktop.search_count, | ||
| treatment=BootstrapMean(num_samples=1000), | ||
| ), |
There was a problem hiding this comment.
We could also move those into a .toml config file that lives in the pensieve repository instead of hardcode them. It might be easier to find and change if there is a separate file for this and could also serve as a configuration example.
| of the experiment. | ||
| """ | ||
|
|
||
| ref_branch_label: str = "control" |
There was a problem hiding this comment.
Is it fair to assume that every statistic has ref_branch_label?
There was a problem hiding this comment.
Not every, but many statistics will be interested in making a comparison to the control group.
| if statistic.name() == statistic_name: | ||
| pre_treatments = [pt.resolve for pt in self.pre_treatments] | ||
|
|
||
| if "ref_branch_label" not in params: |
There was a problem hiding this comment.
So unless ref_branch_label is explicitly set in the config we'll use the control branch from experimenter. Or else it's just the default "control" branch
pensieve/pre_treatment.py
Outdated
| """Return snake-cased name of the statistic.""" | ||
| return re.sub(r"(?<!^)(?=[A-Z])", "_", cls.__name__).lower() | ||
|
|
||
| #@abstractmethod |
There was a problem hiding this comment.
mypy doesn't agree with this...
This PR adds support for defining and processing statistics. Currently still a WIP, but I think it's makes sense to see if this is going in the right direction.