From f88d7974fc5386c0b404fbfaea7f7e013c21d405 Mon Sep 17 00:00:00 2001 From: Valeria Kerol Date: Fri, 6 Sep 2024 12:31:33 +0200 Subject: [PATCH 1/4] add trigger to filters --- src/epstats/server/req.py | 18 ++++++++-- src/epstats/toolkit/experiment.py | 6 ++++ tests/epstats/server/test_req.py | 57 +++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/epstats/server/req.py b/src/epstats/server/req.py index 698fc69..18541bf 100644 --- a/src/epstats/server/req.py +++ b/src/epstats/server/req.py @@ -153,9 +153,9 @@ class Filter(BaseModel): Filter specification for data to evaluate. """ - dimension: str = Field(..., title="Name of the dimension") + dimension: Optional[str] = Field(..., title="Name of the dimension") - value: List[Any] = Field(..., title="List of possible values") + value: Optional[List[Any]] = Field(..., title="List of possible values") scope: FilterScope = Field( ..., @@ -163,8 +163,20 @@ class Filter(BaseModel): description="Scope of the filter is either `exposure` or `goal`.", ) + goal: Optional[str] = Field(None, title="Specify goals if filter scope is `trigger`") + + @model_validator(mode="after") + def check_trigger(self): + if self.scope == FilterScope.trigger: + if not self.goal: + raise ValueError("Goal is required for scope 'trigger'") + else: + if not (self.dimension or self.value): + raise ValueError("Dimension and value are required for this scope") + return self + def to_filter(self): - return EvFilter(self.dimension, self.value, self.scope) + return EvFilter(self.dimension, self.value, self.scope, self.goal) class Experiment(BaseModel): diff --git a/src/epstats/toolkit/experiment.py b/src/epstats/toolkit/experiment.py index 98fc5fe..e1d18e7 100644 --- a/src/epstats/toolkit/experiment.py +++ b/src/epstats/toolkit/experiment.py @@ -112,6 +112,7 @@ class FilterScope(str, Enum): exposure = "exposure" goal = "goal" + trigger = "trigger" @dataclass @@ -123,6 +124,11 @@ class Filter: dimension: str value: List[Any] scope: FilterScope + goal: Optional[str] = None + + def __post_init__(self): + if self.scope == FilterScope.trigger and not self.goal: + raise ValueError("Trigger scope requires goal") class Experiment: diff --git a/tests/epstats/server/test_req.py b/tests/epstats/server/test_req.py index 8926ee6..96ac9d6 100644 --- a/tests/epstats/server/test_req.py +++ b/tests/epstats/server/test_req.py @@ -257,3 +257,60 @@ def test_validate_date_for_between_date_to_and_date_for(): json = resp.json() assert json["detail"][0]["loc"][0] == "body" assert json["detail"][0]["type"] == "value_error" + + +def test_filter_scope_trigger_empty_goal(): + json_blob = { + "id": "test-trigger", + "control_variant": "a", + "variants": ["a", "b"], + "unit_type": "test_unit_type", + "filters": [ + {"dimension": "element", "value": ["button-1"], "scope": "trigger"}, + ], + "metrics": [], + "checks": [], + } + + resp = client.post("/evaluate", json=json_blob) + assert resp.status_code == 422 + json = resp.json() + assert json["detail"][0]["loc"][0] == "body" + assert json["detail"][0]["type"] == "value_error" + + +def test_filter_scope_trigger_empty_dimension(): + json_blob = { + "id": "test-trigger", + "control_variant": "a", + "variants": ["a", "b"], + "unit_type": "test_unit_type", + "filters": [ + {"dimension": None, "value": [], "scope": "trigger", "goal": "view"}, + ], + "metrics": [], + "checks": [], + } + + resp = client.post("/evaluate", json=json_blob) + assert resp.status_code == 200 + + +def test_filter_scope_exposure_empty_dimension(): + json_blob = { + "id": "test-trigger", + "control_variant": "a", + "variants": ["a", "b"], + "unit_type": "test_unit_type", + "filters": [ + {"dimension": None, "value": [], "scope": "exposure"}, + ], + "metrics": [], + "checks": [], + } + + resp = client.post("/evaluate", json=json_blob) + assert resp.status_code == 422 + json = resp.json() + assert json["detail"][0]["loc"][0] == "body" + assert json["detail"][0]["type"] == "value_error" From 9c27a9a1966d2757f69391dfe6943635a5dc130f Mon Sep 17 00:00:00 2001 From: Valeria Kerol Date: Fri, 6 Sep 2024 14:05:55 +0200 Subject: [PATCH 2/4] minor changes --- src/epstats/server/req.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/epstats/server/req.py b/src/epstats/server/req.py index 18541bf..92874eb 100644 --- a/src/epstats/server/req.py +++ b/src/epstats/server/req.py @@ -153,16 +153,16 @@ class Filter(BaseModel): Filter specification for data to evaluate. """ - dimension: Optional[str] = Field(..., title="Name of the dimension") - - value: Optional[List[Any]] = Field(..., title="List of possible values") - scope: FilterScope = Field( ..., title="Scope of the filter", description="Scope of the filter is either `exposure` or `goal`.", ) + dimension: Optional[str] = Field(None, title="Name of the dimension") + + value: Optional[List[Any]] = Field(None, title="List of possible values") + goal: Optional[str] = Field(None, title="Specify goals if filter scope is `trigger`") @model_validator(mode="after") From 1bc6d8081149d99c20b956b9e7a674a1759a2d89 Mon Sep 17 00:00:00 2001 From: Valeria Kerol Date: Fri, 6 Sep 2024 14:51:17 +0200 Subject: [PATCH 3/4] additinal validation in Filter --- src/epstats/server/req.py | 2 +- src/epstats/toolkit/experiment.py | 12 ++++++++---- tests/epstats/toolkit/test_experiment.py | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/epstats/server/req.py b/src/epstats/server/req.py index 92874eb..0336672 100644 --- a/src/epstats/server/req.py +++ b/src/epstats/server/req.py @@ -176,7 +176,7 @@ def check_trigger(self): return self def to_filter(self): - return EvFilter(self.dimension, self.value, self.scope, self.goal) + return EvFilter(self.scope, self.dimension, self.value, self.goal) class Experiment(BaseModel): diff --git a/src/epstats/toolkit/experiment.py b/src/epstats/toolkit/experiment.py index e1d18e7..20b9a40 100644 --- a/src/epstats/toolkit/experiment.py +++ b/src/epstats/toolkit/experiment.py @@ -121,14 +121,18 @@ class Filter: Filter specification for data to evaluate. """ - dimension: str - value: List[Any] scope: FilterScope + dimension: Optional[str] = None + value: Optional[List[Any]] = None goal: Optional[str] = None def __post_init__(self): - if self.scope == FilterScope.trigger and not self.goal: - raise ValueError("Trigger scope requires goal") + if self.scope == FilterScope.trigger: + if not self.goal: + raise ValueError("Trigger scope requires goal") + else: + if not (self.dimension or self.value): + raise ValueError("Dimension and value are required for this scope") class Experiment: diff --git a/tests/epstats/toolkit/test_experiment.py b/tests/epstats/toolkit/test_experiment.py index dd04b55..26f27e3 100644 --- a/tests/epstats/toolkit/test_experiment.py +++ b/tests/epstats/toolkit/test_experiment.py @@ -410,7 +410,7 @@ def test_filter_scope_goal(dao, metrics, checks, unit_type): [SrmCheck(1, "SRM", "count(test_unit_type.global.exposure)")], unit_type=unit_type, variants=["a", "b"], - filters=[Filter("element", ["button-1"], FilterScope.goal)], + filters=[Filter(FilterScope.goal, "element", ["button-1"])], ) evaluate_experiment_agg(experiment, dao) From 7665719a609e89fd5203d55664aba0bb647d96cc Mon Sep 17 00:00:00 2001 From: Valeria Kerol Date: Fri, 6 Sep 2024 15:05:20 +0200 Subject: [PATCH 4/4] update --- src/epstats/server/req.py | 2 +- src/epstats/toolkit/experiment.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/epstats/server/req.py b/src/epstats/server/req.py index 0336672..87628cc 100644 --- a/src/epstats/server/req.py +++ b/src/epstats/server/req.py @@ -171,7 +171,7 @@ def check_trigger(self): if not self.goal: raise ValueError("Goal is required for scope 'trigger'") else: - if not (self.dimension or self.value): + if not (self.dimension and self.value): raise ValueError("Dimension and value are required for this scope") return self diff --git a/src/epstats/toolkit/experiment.py b/src/epstats/toolkit/experiment.py index 20b9a40..79c50ff 100644 --- a/src/epstats/toolkit/experiment.py +++ b/src/epstats/toolkit/experiment.py @@ -131,7 +131,7 @@ def __post_init__(self): if not self.goal: raise ValueError("Trigger scope requires goal") else: - if not (self.dimension or self.value): + if not (self.dimension and self.value): raise ValueError("Dimension and value are required for this scope")