From 2231ef4235638aedd6b02b7ad09c251ed97463b2 Mon Sep 17 00:00:00 2001 From: Jeff Date: Thu, 23 Sep 2021 14:00:06 -0700 Subject: [PATCH 1/8] assert float feature is still float from online store Signed-off-by: Jeff --- .../tests/integration/online_store/test_e2e_local.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/integration/online_store/test_e2e_local.py b/sdk/python/tests/integration/online_store/test_e2e_local.py index 6a3bc02d2f2..dd900e90dc0 100644 --- a/sdk/python/tests/integration/online_store/test_e2e_local.py +++ b/sdk/python/tests/integration/online_store/test_e2e_local.py @@ -27,7 +27,7 @@ def _assert_online_features( ): """Assert that features in online store are up to date with `max_date` date.""" # Read features back - result = store.get_online_features( + response = store.get_online_features( features=[ "driver_hourly_stats:conv_rate", "driver_hourly_stats:avg_daily_trips", @@ -36,8 +36,14 @@ def _assert_online_features( ], entity_rows=[{"driver_id": 1001}], full_feature_names=True, - ).to_dict() + ) + + # Float features should still be floats from the online store... + assert ( + response.field_values[0].fields["driver_hourly_stats__conv_rate"].float_val > 0 + ) + result = response.to_dict() assert len(result) == 5 assert "driver_hourly_stats__avg_daily_trips" in result assert "driver_hourly_stats__conv_rate" in result From eb2bcfd36ce6843319a2f8b34fc9a8e5f43c2a79 Mon Sep 17 00:00:00 2001 From: Jeff Date: Thu, 23 Sep 2021 15:40:40 -0700 Subject: [PATCH 2/8] ensure float features retain float type from online store Floats were converted to doubles when materialized to the online store. There is a broader bug trend around type conversions and this particular conversion utility function looks like it could use some cleanup. This commit is a quick fix. Signed-off-by: Jeff --- sdk/python/feast/type_map.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py index 4e015307927..33e0f360847 100644 --- a/sdk/python/feast/type_map.py +++ b/sdk/python/feast/type_map.py @@ -325,7 +325,11 @@ def python_value_to_proto_value( else python_type_to_feast_value_type("", value) ) else: - value_type = python_type_to_feast_value_type("", value) + value_type = ( + feature_type + if feature_type is ValueType.FLOAT + else python_type_to_feast_value_type("", value) + ) return _python_value_to_proto_value(value_type, value) From 2819dd355848279379da494e62fd0425ede56ab8 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 28 Sep 2021 13:54:42 -0700 Subject: [PATCH 3/8] make fix more general Signed-off-by: Achal Shah --- sdk/python/feast/type_map.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py index 33e0f360847..02161cbca18 100644 --- a/sdk/python/feast/type_map.py +++ b/sdk/python/feast/type_map.py @@ -317,7 +317,7 @@ def python_value_to_proto_value( value: Any, feature_type: ValueType = ValueType.UNKNOWN ) -> ProtoValue: value_type = feature_type - if value is not None: + if value is not None and feature_type == ValueType.UNKNOWN: if isinstance(value, (list, np.ndarray)): value_type = ( feature_type @@ -325,11 +325,7 @@ def python_value_to_proto_value( else python_type_to_feast_value_type("", value) ) else: - value_type = ( - feature_type - if feature_type is ValueType.FLOAT - else python_type_to_feast_value_type("", value) - ) + value_type = python_type_to_feast_value_type("", value) return _python_value_to_proto_value(value_type, value) From 8d270e85721ef8d4892d9818c95cf1182c6fd94e Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 28 Sep 2021 14:36:35 -0700 Subject: [PATCH 4/8] Use assertAlmostEquals Signed-off-by: Achal Shah --- .../online_store/test_universal_online.py | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index e6543ae54af..30fa36c0f24 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -110,24 +110,26 @@ def test_online_retrieval(environment, universal_data_sources, full_feature_name assert df_features["customer_id"] == online_features_dict["customer_id"][i] assert df_features["driver_id"] == online_features_dict["driver_id"][i] - assert ( + tc.assertAlmostEqual( online_features_dict[ response_feature_name("conv_rate_plus_100", full_feature_names) - ][i] - == df_features["conv_rate"] + 100 + ][i], + df_features["conv_rate"] + 100, + delta=0.0001 ) - assert ( + tc.assertAlmostEqual( online_features_dict[ response_feature_name("conv_rate_plus_val_to_add", full_feature_names) - ][i] - == df_features["conv_rate"] + df_features["val_to_add"] - ) + ][i], + df_features["conv_rate"] + df_features["val_to_add"], + delta=0.0001) for unprefixed_feature_ref in unprefixed_feature_refs: - tc.assertEqual( + tc.assertAlmostEqual( df_features[unprefixed_feature_ref], online_features_dict[ response_feature_name(unprefixed_feature_ref, full_feature_names) ][i], + delta=0.0001 ) # Check what happens for missing values @@ -254,13 +256,15 @@ def assert_feature_service_correctness( + 3 ) # Add two for the driver id and the customer id entity keys and val_to_add request data + tc = unittest.TestCase() for i, entity_row in enumerate(entity_rows): df_features = get_latest_feature_values_from_dataframes( drivers_df, customers_df, orders_df, global_df, entity_row ) - assert ( + tc.assertAlmostEqual( feature_service_online_features_dict[ response_feature_name("conv_rate_plus_100", full_feature_names) - ][i] - == df_features["conv_rate"] + 100 + ][i], + df_features["conv_rate"] + 100, + delta=0.0001 ) From 07a1aa221d1ff29ad2774005ec0fae5876b6417b Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 28 Sep 2021 14:45:07 -0700 Subject: [PATCH 5/8] format Signed-off-by: Achal Shah --- .../integration/online_store/test_universal_online.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index 30fa36c0f24..3f124632e77 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -115,21 +115,22 @@ def test_online_retrieval(environment, universal_data_sources, full_feature_name response_feature_name("conv_rate_plus_100", full_feature_names) ][i], df_features["conv_rate"] + 100, - delta=0.0001 + delta=0.0001, ) tc.assertAlmostEqual( online_features_dict[ response_feature_name("conv_rate_plus_val_to_add", full_feature_names) ][i], df_features["conv_rate"] + df_features["val_to_add"], - delta=0.0001) + delta=0.0001, + ) for unprefixed_feature_ref in unprefixed_feature_refs: tc.assertAlmostEqual( df_features[unprefixed_feature_ref], online_features_dict[ response_feature_name(unprefixed_feature_ref, full_feature_names) ][i], - delta=0.0001 + delta=0.0001, ) # Check what happens for missing values @@ -266,5 +267,5 @@ def assert_feature_service_correctness( response_feature_name("conv_rate_plus_100", full_feature_names) ][i], df_features["conv_rate"] + 100, - delta=0.0001 + delta=0.0001, ) From 60574af10371f5a059f3b71934e891124a2302b3 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 28 Sep 2021 16:43:51 -0700 Subject: [PATCH 6/8] Support pandas timestamps correctly Signed-off-by: Achal Shah --- sdk/python/feast/type_map.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py index 02161cbca18..336981eccdf 100644 --- a/sdk/python/feast/type_map.py +++ b/sdk/python/feast/type_map.py @@ -19,9 +19,9 @@ import numpy as np import pandas as pd import pyarrow +from google.protobuf.internal.well_known_types import Timestamp from google.protobuf.json_format import MessageToDict from google.protobuf.pyext.cpp_message import GeneratedProtocolMessageType -from google.protobuf.timestamp_pb2 import Timestamp from feast.protos.feast.types.Value_pb2 import ( BoolList, @@ -245,7 +245,7 @@ def _type_err(item, dtype): ValueType, Tuple[str, Any, Optional[Set[Type]]] ] = { ValueType.INT32: ("int32_val", lambda x: int(x), None), - ValueType.INT64: ("int64_val", lambda x: int(x), None), + ValueType.INT64: ("int64_val", lambda x: int(x.timestamp()) if isinstance(x, pd._libs.tslibs.timestamps.Timestamp) else int(x), None), ValueType.FLOAT: ("float_val", lambda x: float(x), None), ValueType.DOUBLE: ("double_val", lambda x: x, {float, np.float64}), ValueType.STRING: ("string_val", lambda x: str(x), None), From e13eae6720429bf35671ea89979be66f3d86e615 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 28 Sep 2021 16:44:51 -0700 Subject: [PATCH 7/8] Support pandas timestamps correctly Signed-off-by: Achal Shah --- sdk/python/feast/type_map.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py index 336981eccdf..fe1b1988066 100644 --- a/sdk/python/feast/type_map.py +++ b/sdk/python/feast/type_map.py @@ -245,7 +245,13 @@ def _type_err(item, dtype): ValueType, Tuple[str, Any, Optional[Set[Type]]] ] = { ValueType.INT32: ("int32_val", lambda x: int(x), None), - ValueType.INT64: ("int64_val", lambda x: int(x.timestamp()) if isinstance(x, pd._libs.tslibs.timestamps.Timestamp) else int(x), None), + ValueType.INT64: ( + "int64_val", + lambda x: int(x.timestamp()) + if isinstance(x, pd._libs.tslibs.timestamps.Timestamp) + else int(x), + None, + ), ValueType.FLOAT: ("float_val", lambda x: float(x), None), ValueType.DOUBLE: ("double_val", lambda x: x, {float, np.float64}), ValueType.STRING: ("string_val", lambda x: str(x), None), From badcdf8c29dcae53cc728e149bbeeabe7bca07e4 Mon Sep 17 00:00:00 2001 From: Achal Shah Date: Tue, 28 Sep 2021 16:49:17 -0700 Subject: [PATCH 8/8] Correct import Signed-off-by: Achal Shah --- sdk/python/feast/type_map.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py index fe1b1988066..79fd7ed1b10 100644 --- a/sdk/python/feast/type_map.py +++ b/sdk/python/feast/type_map.py @@ -19,9 +19,9 @@ import numpy as np import pandas as pd import pyarrow -from google.protobuf.internal.well_known_types import Timestamp from google.protobuf.json_format import MessageToDict from google.protobuf.pyext.cpp_message import GeneratedProtocolMessageType +from google.protobuf.timestamp_pb2 import Timestamp from feast.protos.feast.types.Value_pb2 import ( BoolList,