Skip to content

Commit 788d66c

Browse files
Alda/numeric metrics (#76)
* Added a helper method to find numeric metric values in the event tags. * Added tests for the numeric metric value helper method. * Renamed REVENUE_METRIC_TYPE enum to EVENT_VALUE_METRIC to be consistent with Java. * Replaced hard-coded revenue tag with enum. Renamed the local variable event value to revenue value to better describe its reference. * Added a check for numeric metric value in the event builder. * Fixed unit tests for numeric metric. * Changed event value test names to event tags. Updated test comment descriptions. * Added a couple more unit tests to test track and conversion event generation are valid when only revenue or numeric metric event tags are provided. * Fixed lint errors. * Enabled max diff to see unit test error on travis. * Fixed nit spacing and wording. * Sort the event tag keys as they were being returned in an undeterministic way, which made some unit tests inconsistent. Event tag keys is a short list, so the sort should not create overhead. * Refactored numeric metric unit tests to use a subroutine and isolated event object checks to parameters related to event tags so that the unit tests won't be brittle. * Handled corner cases for the get numeric value better. Strings and numbers are supported; everything else should return None. * Sorted the event features results in the test so we don't have to sort it in the event builder. * Fixed an import statement. * Removed basestring as this is not supported by Python3. * Updated some code comments. * Cleaned up some wording and unit test ordering. * Removed blank line at end of file.
1 parent 70e1d44 commit 788d66c

File tree

6 files changed

+562
-92
lines changed

6 files changed

+562
-92
lines changed

optimizely/event_builder.py

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,23 @@ def _add_required_params_for_conversion(self, event_key, event_tags, decisions):
197197
self.params[self.EventParams.EVENT_METRICS] = []
198198

199199
if event_tags:
200-
event_value = event_tag_utils.get_revenue_value(event_tags)
201-
if event_value is not None:
202-
self.params[self.EventParams.EVENT_METRICS] = [{
203-
'name': event_tag_utils.EVENT_VALUE_METRIC,
204-
'value': event_value
205-
}]
206-
200+
event_values = []
201+
202+
revenue_value = event_tag_utils.get_revenue_value(event_tags)
203+
if revenue_value is not None:
204+
event_values.append({
205+
'name': event_tag_utils.REVENUE_METRIC_TYPE,
206+
'value': revenue_value
207+
})
208+
209+
numeric_value = event_tag_utils.get_numeric_value(event_tags, self.config.logger)
210+
if numeric_value is not None:
211+
event_values.append({
212+
'name': event_tag_utils.NUMERIC_METRIC_TYPE,
213+
'value': numeric_value
214+
})
215+
216+
self.params[self.EventParams.EVENT_METRICS] = event_values
207217
for event_tag_id in event_tags.keys():
208218
event_tag_value = event_tags.get(event_tag_id)
209219
if event_tag_value is None:
@@ -422,9 +432,13 @@ def _add_required_params_for_conversion(self, event_key, event_tags, decisions):
422432
}
423433

424434
if event_tags:
425-
event_value = event_tag_utils.get_revenue_value(event_tags)
426-
if event_value is not None:
427-
event_dict['revenue'] = event_value
435+
revenue_value = event_tag_utils.get_revenue_value(event_tags)
436+
if revenue_value is not None:
437+
event_dict[event_tag_utils.REVENUE_METRIC_TYPE] = revenue_value
438+
439+
numeric_value = event_tag_utils.get_numeric_value(event_tags, self.config.logger)
440+
if numeric_value is not None:
441+
event_dict[event_tag_utils.NUMERIC_METRIC_TYPE] = numeric_value
428442

429443
if len(event_tags) > 0:
430444
event_dict[self.EventParams.TAGS] = event_tags

optimizely/helpers/event_tag_utils.py

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@
1111
# See the License for the specific language governing permissions and
1212
# limitations under the License.
1313

14+
from . import enums
15+
import math
1416
import numbers
1517

16-
EVENT_VALUE_METRIC = 'revenue'
18+
REVENUE_METRIC_TYPE = 'revenue'
19+
NUMERIC_METRIC_TYPE = 'value'
1720

1821

1922
def get_revenue_value(event_tags):
@@ -23,12 +26,94 @@ def get_revenue_value(event_tags):
2326
if not isinstance(event_tags, dict):
2427
return None
2528

26-
if EVENT_VALUE_METRIC not in event_tags:
29+
if REVENUE_METRIC_TYPE not in event_tags:
2730
return None
2831

29-
raw_value = event_tags[EVENT_VALUE_METRIC]
32+
raw_value = event_tags[REVENUE_METRIC_TYPE]
3033

3134
if not isinstance(raw_value, numbers.Integral):
3235
return None
3336

3437
return raw_value
38+
39+
40+
def get_numeric_value(event_tags, logger=None):
41+
"""
42+
A smart getter of the numeric value from the event tags.
43+
44+
Args:
45+
event_tags: A dictionary of event tags.
46+
logger: Optional logger.
47+
48+
Returns:
49+
A float numeric metric value is returned when the provided numeric
50+
metric value is in the following format:
51+
- A string (properly formatted, e.g., no commas)
52+
- An integer
53+
- A float or double
54+
None is returned when the provided numeric metric values is in
55+
the following format:
56+
- None
57+
- A boolean
58+
- inf, -inf, nan
59+
- A string not properly formatted (e.g., '1,234')
60+
- Any values that cannot be cast to a float (e.g., an array or dictionary)
61+
"""
62+
63+
logger_message_debug = None
64+
numeric_metric_value = None
65+
66+
if event_tags is None:
67+
logger_message_debug = 'Event tags is undefined.'
68+
elif not isinstance(event_tags, dict):
69+
logger_message_debug = 'Event tags is not a dictionary.'
70+
elif NUMERIC_METRIC_TYPE not in event_tags:
71+
logger_message_debug = 'The numeric metric key is not in event tags.'
72+
else:
73+
numeric_metric_value = event_tags[NUMERIC_METRIC_TYPE]
74+
try:
75+
if isinstance(numeric_metric_value, (numbers.Integral, float, str)):
76+
# Attempt to convert the numeric metric value to a float
77+
# (if it isn't already a float).
78+
cast_numeric_metric_value = float(numeric_metric_value)
79+
80+
# If not a float after casting, then make everything else a None.
81+
# Other potential values are nan, inf, and -inf.
82+
if not isinstance(cast_numeric_metric_value, float) \
83+
or math.isnan(cast_numeric_metric_value) \
84+
or math.isinf(cast_numeric_metric_value):
85+
logger_message_debug = 'Provided numeric value {} is in an invalid format.'\
86+
.format(numeric_metric_value)
87+
numeric_metric_value = None
88+
else:
89+
# Handle booleans as a special case.
90+
# They are treated like an integer in the cast, but we do not want to cast this.
91+
if isinstance(numeric_metric_value, bool):
92+
logger_message_debug = 'Provided numeric value is a boolean, which is an invalid format.'
93+
numeric_metric_value = None
94+
else:
95+
numeric_metric_value = cast_numeric_metric_value
96+
else:
97+
logger_message_debug = 'Numeric metric value is not in integer, float, or string form.'
98+
99+
except ValueError:
100+
logger_message_debug = 'Value error while casting numeric metric value to a float.'
101+
numeric_metric_value = None
102+
103+
# Log all potential debug messages while converting the numeric value to a float.
104+
if logger and logger_message_debug:
105+
logger.log(enums.LogLevels.DEBUG, logger_message_debug)
106+
107+
# Log the final numeric metric value
108+
if numeric_metric_value is not None:
109+
if logger:
110+
logger.log(enums.LogLevels.INFO,
111+
'The numeric metric value {} will be sent to results.'
112+
.format(numeric_metric_value))
113+
else:
114+
if logger:
115+
logger.log(enums.LogLevels.WARNING,
116+
'The provided numeric metric value {} is in an invalid format and will not be sent to results.'
117+
.format(numeric_metric_value))
118+
119+
return numeric_metric_value

optimizely/optimizely.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from .error_handler import NoOpErrorHandler as noop_error_handler
2222
from .event_dispatcher import EventDispatcher as default_event_dispatcher
2323
from .helpers import enums
24+
from .helpers import event_tag_utils
2425
from .helpers import validator
2526
from .logger import NoOpLogger as noop_logger
2627
from .logger import SimpleLogger

tests/helpers_tests/test_event_tag_utils.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
# See the License for the specific language governing permissions and
1212
# limitations under the License.
1313

14+
import sys
1415
import unittest
16+
from optimizely import logger
1517

1618
from optimizely.helpers import event_tag_utils
1719

@@ -49,3 +51,78 @@ def test_get_revenue_value__revenue_tag(self):
4951
self.assertEqual(0, event_tag_utils.get_revenue_value({'revenue': 0}))
5052
self.assertEqual(65536, event_tag_utils.get_revenue_value({'revenue': 65536}))
5153
self.assertEqual(9223372036854775807, event_tag_utils.get_revenue_value({'revenue': 9223372036854775807}))
54+
55+
def test_get_numeric_metric__invalid_args(self):
56+
""" Test that numeric value is not returned for invalid arguments. """
57+
self.assertIsNone(event_tag_utils.get_numeric_value(None))
58+
self.assertIsNone(event_tag_utils.get_numeric_value(0.5))
59+
self.assertIsNone(event_tag_utils.get_numeric_value(65536))
60+
self.assertIsNone(event_tag_utils.get_numeric_value(9223372036854775807))
61+
self.assertIsNone(event_tag_utils.get_numeric_value('9223372036854775807'))
62+
self.assertIsNone(event_tag_utils.get_numeric_value(True))
63+
self.assertIsNone(event_tag_utils.get_numeric_value(False))
64+
65+
def test_get_numeric_metric__no_value_tag(self):
66+
""" Test that numeric value is not returned when there's no numeric event tag. """
67+
self.assertIsNone(event_tag_utils.get_numeric_value([]))
68+
self.assertIsNone(event_tag_utils.get_numeric_value({}))
69+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': 42}))
70+
71+
def test_get_numeric_metric__invalid_value_tag(self):
72+
""" Test that numeric value is not returned when revenue event tag has invalid data type. """
73+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': None}))
74+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': 0.5}))
75+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': 12345}))
76+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': '65536'}))
77+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': True}))
78+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': False}))
79+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': [1, 2, 3]}))
80+
self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': {'a', 'b', 'c'}}))
81+
82+
def test_get_numeric_metric__value_tag(self):
83+
""" Test that the correct numeric value is returned. """
84+
85+
# An integer should be cast to a float
86+
self.assertEqual(12345.0, event_tag_utils.get_numeric_value({'value': 12345}, logger=logger.SimpleLogger()))
87+
88+
# A string should be cast to a float
89+
self.assertEqual(12345.0, event_tag_utils.get_numeric_value({'value': '12345'}, logger=logger.SimpleLogger()))
90+
91+
# Valid float values
92+
some_float = 1.2345
93+
self.assertEqual(some_float, event_tag_utils.get_numeric_value({'value': some_float}, logger=logger.SimpleLogger()))
94+
95+
max_float = sys.float_info.max
96+
self.assertEqual(max_float, event_tag_utils.get_numeric_value({'value': max_float}, logger=logger.SimpleLogger()))
97+
98+
min_float = sys.float_info.min
99+
self.assertEqual(min_float, event_tag_utils.get_numeric_value({'value': min_float}, logger=logger.SimpleLogger()))
100+
101+
# Invalid values
102+
self.assertIsNone(event_tag_utils.get_numeric_value({'value': False}, logger=logger.SimpleLogger()))
103+
self.assertIsNone(event_tag_utils.get_numeric_value({'value': None}, logger=logger.SimpleLogger()))
104+
105+
numeric_value_nan = event_tag_utils.get_numeric_value({'value': float('nan')}, logger=logger.SimpleLogger())
106+
self.assertIsNone(numeric_value_nan, 'nan numeric value is {}'.format(numeric_value_nan))
107+
108+
numeric_value_array = event_tag_utils.get_numeric_value({'value': []}, logger=logger.SimpleLogger())
109+
self.assertIsNone(numeric_value_nan, 'Array numeric value is {}'.format(numeric_value_array))
110+
111+
numeric_value_none = event_tag_utils.get_numeric_value({'value': None}, logger=logger.SimpleLogger())
112+
self.assertIsNone(numeric_value_none, 'None numeric value is {}'.format(numeric_value_none))
113+
114+
numeric_value_invalid_literal = event_tag_utils.get_numeric_value({'value': '1,234'}, logger=logger.SimpleLogger())
115+
self.assertIsNone(numeric_value_invalid_literal, 'Invalid string literal value is {}'
116+
.format(numeric_value_invalid_literal))
117+
118+
numeric_value_overflow = event_tag_utils.get_numeric_value({'value': sys.float_info.max * 10},
119+
logger=logger.SimpleLogger())
120+
self.assertIsNone(numeric_value_overflow, 'Max numeric value is {}'.format(numeric_value_overflow))
121+
122+
numeric_value_inf = event_tag_utils.get_numeric_value({'value': float('inf')}, logger=logger.SimpleLogger())
123+
self.assertIsNone(numeric_value_inf, 'Infinity numeric value is {}'.format(numeric_value_inf))
124+
125+
numeric_value_neg_inf = event_tag_utils.get_numeric_value({'value': float('-inf')}, logger=logger.SimpleLogger())
126+
self.assertIsNone(numeric_value_neg_inf, 'Negative infinity numeric value is {}'.format(numeric_value_neg_inf))
127+
128+
self.assertEqual(0.0, event_tag_utils.get_numeric_value({'value': 0.0}, logger=logger.SimpleLogger()))

0 commit comments

Comments
 (0)