From 8ef8ec35f183488f23728870b70be46c3a1f72d0 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Tue, 11 Jun 2024 14:13:14 -0700 Subject: [PATCH 1/3] fix tests --- test/client_bandit_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/client_bandit_test.py b/test/client_bandit_test.py index c870219..d8a1e6a 100644 --- a/test/client_bandit_test.py +++ b/test/client_bandit_test.py @@ -71,6 +71,7 @@ def init_fixture(): base_url=MOCK_BASE_URL, api_key="dummy", assignment_logger=mock_assignment_logger, + is_graceful_mode=False, ) ) sleep(0.1) # wait for initialization @@ -91,7 +92,7 @@ def test_get_bandit_action_bandit_does_not_exist(): "nonexistent_bandit", "subject_key", DEFAULT_SUBJECT_ATTRIBUTES, - [], + {}, "default_variation", ) assert result == BanditResult("default_variation", None) @@ -100,7 +101,7 @@ def test_get_bandit_action_bandit_does_not_exist(): def test_get_bandit_action_flag_without_bandit(): client = get_instance() result = client.get_bandit_action( - "a_flag", "subject_key", DEFAULT_SUBJECT_ATTRIBUTES, [], "default_variation" + "a_flag", "subject_key", DEFAULT_SUBJECT_ATTRIBUTES, {}, "default_variation" ) assert result == BanditResult("default_variation", None) From d1c5a8b4095c8ebb2f9fdbee3446079f36a5d82a Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Tue, 11 Jun 2024 14:13:41 -0700 Subject: [PATCH 2/3] allow attributes to be specified as dicts --- eppo_client/bandit.py | 35 +++++++++++++++++++- eppo_client/client.py | 74 +++++++++++++++++++++++++++++-------------- eppo_client/eval.py | 8 ++--- eppo_client/rules.py | 6 ++-- eppo_client/types.py | 2 +- 5 files changed, 93 insertions(+), 32 deletions(-) diff --git a/eppo_client/bandit.py b/eppo_client/bandit.py index 0529351..bce9c99 100644 --- a/eppo_client/bandit.py +++ b/eppo_client/bandit.py @@ -1,6 +1,6 @@ from dataclasses import dataclass import logging -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Union from eppo_client.models import ( BanditCategoricalAttributeCoefficient, @@ -8,7 +8,9 @@ BanditModelData, BanditNumericAttributeCoefficient, ) +from eppo_client.rules import to_string from eppo_client.sharders import Sharder +from eppo_client.types import AttributesDict logger = logging.getLogger(__name__) @@ -25,10 +27,41 @@ class Attributes: @classmethod def empty(cls): + """ + Create an empty Attributes instance with no numeric or categorical attributes. + + Returns: + Attributes: An instance of the Attributes class with empty dictionaries for numeric and categorical attributes. + """ return cls({}, {}) + @classmethod + def from_dict(cls, attributes: AttributesDict): + """ + Create an Attributes instance from a dictionary of attributes. + + Args: + attributes (Dict[str, Union[float, int, bool, str]]): A dictionary where keys are attribute names + and values are attribute values which can be of type float, int, bool, or str. + + Returns: + Attributes: An instance of the Attributes class with numeric and categorical attributes separated. + """ + numeric_attributes = { + key: float(value) + for key, value in attributes.items() + if isinstance(value, (int, float)) + } + categorical_attributes = { + key: to_string(value) + for key, value in attributes.items() + if isinstance(value, (str, bool)) + } + return cls(numeric_attributes, categorical_attributes) + ActionContexts = Dict[str, Attributes] +ActionContextsDict = Dict[str, AttributesDict] @dataclass diff --git a/eppo_client/client.py b/eppo_client/client.py index 789e185..d1a017c 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -1,9 +1,15 @@ import datetime import logging import json -from typing import Any, Dict, Optional +from typing import Any, Dict, Optional, Union from eppo_client.assignment_logger import AssignmentLogger -from eppo_client.bandit import BanditEvaluator, BanditResult, Attributes, ActionContexts +from eppo_client.bandit import ( + ActionContextsDict, + BanditEvaluator, + BanditResult, + Attributes, + ActionContexts, +) from eppo_client.configuration_requestor import ( ExperimentConfigurationRequestor, ) @@ -11,7 +17,7 @@ from eppo_client.models import VariationType from eppo_client.poller import Poller from eppo_client.sharders import MD5Sharder -from eppo_client.types import SubjectAttributes, ValueType +from eppo_client.types import AttributesDict, ValueType from eppo_client.validation import validate_not_blank from eppo_client.eval import FlagEvaluation, Evaluator, none_result from eppo_client.version import __version__ @@ -43,7 +49,7 @@ def get_string_assignment( self, flag_key: str, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, default: str, ) -> str: return self.get_assignment_variation( @@ -58,7 +64,7 @@ def get_integer_assignment( self, flag_key: str, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, default: int, ) -> int: return self.get_assignment_variation( @@ -73,7 +79,7 @@ def get_numeric_assignment( self, flag_key: str, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, default: float, ) -> float: # convert to float in case we get an int @@ -91,7 +97,7 @@ def get_boolean_assignment( self, flag_key: str, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, default: bool, ) -> bool: return self.get_assignment_variation( @@ -106,7 +112,7 @@ def get_json_assignment( self, flag_key: str, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, default: Dict[Any, Any], ) -> Dict[Any, Any]: json_value = self.get_assignment_variation( @@ -125,7 +131,7 @@ def get_assignment_variation( self, flag_key: str, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, default: Optional[ValueType], expected_variation_type: VariationType, ): @@ -149,7 +155,7 @@ def get_assignment_detail( self, flag_key: str, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, expected_variation_type: VariationType, ) -> FlagEvaluation: """Maps a subject to a variation for a given flag @@ -225,8 +231,8 @@ def get_bandit_action( self, flag_key: str, subject_key: str, - subject_context: Attributes, - actions: ActionContexts, + subject_context: Union[Attributes, AttributesDict], + actions: Union[ActionContexts, ActionContextsDict], default: str, ) -> BanditResult: """ @@ -244,9 +250,11 @@ def get_bandit_action( Args: flag_key (str): The feature flag key that contains the bandit as one of the variations. subject_key (str): The key identifying the subject. - subject_context (Attributes): The subject context - actions (Dict[str, Attributes]): The dictionary that maps action keys + subject_context (Attributes | AttributesDict): The subject context. + If supplying an AttributesDict, it gets converted to an Attributes instance + actions (ActionContexts | ActionContextsDict): The dictionary that maps action keys to their context of actions with their contexts. + If supplying an AttributesDict, it gets converted to an Attributes instance. default (str): The default variation to use if the subject is not part of the bandit. Returns: @@ -264,7 +272,8 @@ def get_bandit_action( categorical_attributes={"country": "USA"}), { "action1": Attributes(numeric_attributes={"price": 10.0}, categorical_attributes={"category": "A"}), - "action2": Attributes.empty() + "action2": {"price": 10.0, "category": "B"} + "action3": Attributes.empty(), }, "default" ) @@ -273,7 +282,6 @@ def get_bandit_action( else: do_action(result.action) """ - try: return self.get_bandit_action_detail( flag_key, @@ -292,14 +300,17 @@ def get_bandit_action_detail( self, flag_key: str, subject_key: str, - subject_context: Attributes, - actions: ActionContexts, + subject_context: Union[Attributes, AttributesDict], + actions: Union[ActionContexts, ActionContextsDict], default: str, ) -> BanditResult: + subject_attributes = convert_subject_context_to_attributes(subject_context) + action_contexts = convert_actions_to_action_contexts(actions) + # get experiment assignment # ignoring type because Dict[str, str] satisfies Dict[str, str | ...] but mypy does not understand variation = self.get_string_assignment( - flag_key, subject_key, subject_context.categorical_attributes, default # type: ignore + flag_key, subject_key, subject_attributes.categorical_attributes | subject_attributes.numeric_attributes, default # type: ignore ) # if the variation is not the bandit key, then the subject is not allocated in the bandit @@ -318,8 +329,8 @@ def get_bandit_action_detail( evaluation = self.__bandit_evaluator.evaluate_bandit( flag_key, subject_key, - subject_context, - actions, + subject_attributes, + action_contexts, bandit_data.model_data, ) @@ -334,12 +345,12 @@ def get_bandit_action_detail( "modelVersion": bandit_data.model_version if evaluation else None, "timestamp": datetime.datetime.utcnow().isoformat(), "subjectNumericAttributes": ( - subject_context.numeric_attributes + subject_attributes.numeric_attributes if evaluation.subject_attributes else {} ), "subjectCategoricalAttributes": ( - subject_context.categorical_attributes + subject_attributes.categorical_attributes if evaluation.subject_attributes else {} ), @@ -410,3 +421,20 @@ def check_value_type_match( if expected_type == VariationType.BOOLEAN: return isinstance(value, bool) return False + + +def convert_subject_context_to_attributes( + subject_context: Union[Attributes, AttributesDict] +) -> Attributes: + if isinstance(subject_context, dict): + return Attributes.from_dict(subject_context) + return subject_context + + +def convert_actions_to_action_contexts( + actions: Union[ActionContexts, ActionContextsDict] +) -> ActionContexts: + return { + k: Attributes.from_dict(v) if isinstance(v, dict) else v + for k, v in actions.items() + } diff --git a/eppo_client/eval.py b/eppo_client/eval.py index ceaa907..e769b17 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -5,7 +5,7 @@ from dataclasses import dataclass import datetime -from eppo_client.types import SubjectAttributes +from eppo_client.types import AttributesDict @dataclass @@ -13,7 +13,7 @@ class FlagEvaluation: flag_key: str variation_type: VariationType subject_key: str - subject_attributes: SubjectAttributes + subject_attributes: AttributesDict allocation_key: Optional[str] variation: Optional[Variation] extra_logging: Dict[str, str] @@ -28,7 +28,7 @@ def evaluate_flag( self, flag: Flag, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, ) -> FlagEvaluation: if not flag.enabled: return none_result( @@ -93,7 +93,7 @@ def none_result( flag_key: str, variation_type: VariationType, subject_key: str, - subject_attributes: SubjectAttributes, + subject_attributes: AttributesDict, ) -> FlagEvaluation: return FlagEvaluation( flag_key=flag_key, diff --git a/eppo_client/rules.py b/eppo_client/rules.py index a481e2a..fc181bf 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -7,7 +7,7 @@ import semver from eppo_client.models import SdkBaseModel -from eppo_client.types import AttributeType, ConditionValueType, SubjectAttributes +from eppo_client.types import AttributeType, ConditionValueType, AttributesDict class OperatorType(Enum): @@ -32,7 +32,7 @@ class Rule(SdkBaseModel): conditions: List[Condition] -def matches_rule(rule: Rule, subject_attributes: SubjectAttributes) -> bool: +def matches_rule(rule: Rule, subject_attributes: AttributesDict) -> bool: return all( evaluate_condition(condition, subject_attributes) for condition in rule.conditions @@ -40,7 +40,7 @@ def matches_rule(rule: Rule, subject_attributes: SubjectAttributes) -> bool: def evaluate_condition( - condition: Condition, subject_attributes: SubjectAttributes + condition: Condition, subject_attributes: AttributesDict ) -> bool: subject_value = subject_attributes.get(condition.attribute, None) if condition.operator == OperatorType.IS_NULL: diff --git a/eppo_client/types.py b/eppo_client/types.py index 09d69b3..1bd4a10 100644 --- a/eppo_client/types.py +++ b/eppo_client/types.py @@ -3,5 +3,5 @@ ValueType = Union[str, int, float, bool] AttributeType = Union[str, int, float, bool, None] ConditionValueType = Union[AttributeType, List[AttributeType]] -SubjectAttributes = Dict[str, AttributeType] +AttributesDict = Dict[str, AttributeType] Action = str From fda00037f049d9d061f5bdf5f2d1b88594430b91 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Tue, 11 Jun 2024 14:18:27 -0700 Subject: [PATCH 3/3] :broom: --- eppo_client/bandit.py | 5 +++-- eppo_client/client.py | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/eppo_client/bandit.py b/eppo_client/bandit.py index bce9c99..f353fb8 100644 --- a/eppo_client/bandit.py +++ b/eppo_client/bandit.py @@ -1,6 +1,6 @@ from dataclasses import dataclass import logging -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional from eppo_client.models import ( BanditCategoricalAttributeCoefficient, @@ -31,7 +31,8 @@ def empty(cls): Create an empty Attributes instance with no numeric or categorical attributes. Returns: - Attributes: An instance of the Attributes class with empty dictionaries for numeric and categorical attributes. + Attributes: An instance of the Attributes class with empty dictionaries + for numeric and categorical attributes. """ return cls({}, {}) diff --git a/eppo_client/client.py b/eppo_client/client.py index d1a017c..d2318f8 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -310,7 +310,11 @@ def get_bandit_action_detail( # get experiment assignment # ignoring type because Dict[str, str] satisfies Dict[str, str | ...] but mypy does not understand variation = self.get_string_assignment( - flag_key, subject_key, subject_attributes.categorical_attributes | subject_attributes.numeric_attributes, default # type: ignore + flag_key, + subject_key, + subject_attributes.categorical_attributes + | subject_attributes.numeric_attributes, # type: ignore + default, ) # if the variation is not the bandit key, then the subject is not allocated in the bandit