From 738355048f0e078006a5e652d699e5335b4f13f8 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 17 Mar 2024 14:25:16 -0400 Subject: [PATCH 01/27] feat: updating protos to separate transformation Signed-off-by: Francisco Javier Arceo --- protos/feast/core/OnDemandFeatureView.proto | 28 ++------------------- protos/feast/core/Transformation.proto | 12 ++++----- 2 files changed, 7 insertions(+), 33 deletions(-) diff --git a/protos/feast/core/OnDemandFeatureView.proto b/protos/feast/core/OnDemandFeatureView.proto index cd3ceba150..d57465ef96 100644 --- a/protos/feast/core/OnDemandFeatureView.proto +++ b/protos/feast/core/OnDemandFeatureView.proto @@ -49,12 +49,8 @@ message OnDemandFeatureViewSpec { // Map of sources for this feature view. map sources = 4; - oneof transformation { - UserDefinedFunction user_defined_function = 5 [deprecated = true]; - OnDemandSubstraitTransformation on_demand_substrait_transformation = 9 [deprecated = true]; - } // Oneof with {user_defined_function, on_demand_substrait_transformation} - FeatureTransformationV2 feature_transformation = 10; + FeatureTransformation transformation = 5; // Description of the on demand feature view. string description = 6; @@ -64,7 +60,7 @@ message OnDemandFeatureViewSpec { // Owner of the on demand feature view. string owner = 8; - string mode = 11; + string mode = 9; } message OnDemandFeatureViewMeta { @@ -82,23 +78,3 @@ message OnDemandSource { DataSource request_data_source = 2; } } - -// Serialized representation of python function. -message UserDefinedFunction { - option deprecated = true; - - // The function name - string name = 1; - - // The python-syntax function body (serialized by dill) - bytes body = 2; - - // The string representation of the udf - string body_text = 3; -} - -message OnDemandSubstraitTransformation { - option deprecated = true; - - bytes substrait_plan = 1; -} diff --git a/protos/feast/core/Transformation.proto b/protos/feast/core/Transformation.proto index cde2833fa4..2116ec055d 100644 --- a/protos/feast/core/Transformation.proto +++ b/protos/feast/core/Transformation.proto @@ -8,7 +8,7 @@ option java_package = "feast.proto.core"; import "google/protobuf/duration.proto"; // Serialized representation of python function. -message UserDefinedFunctionV2 { +message UserDefinedFunction { // The function name string name = 1; @@ -19,15 +19,13 @@ message UserDefinedFunctionV2 { string body_text = 3; } -// A feature transformation executed as a user-defined function -message FeatureTransformationV2 { - // Note this Transformation starts at 5 for backwards compatibility +message FeatureTransformation { oneof transformation { - UserDefinedFunctionV2 user_defined_function = 1; - OnDemandSubstraitTransformationV2 on_demand_substrait_transformation = 2; + UserDefinedFunction user_defined_function = 1; + OnDemandSubstraitTransformation on_demand_substrait_transformation = 2; } } -message OnDemandSubstraitTransformationV2 { +message OnDemandSubstraitTransformation { bytes substrait_plan = 1; } From 6bcff8d49218047bb18cb316e719050310135e46 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 17 Mar 2024 14:44:21 -0400 Subject: [PATCH 02/27] fixed stuff...i think Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/diff/registry_diff.py | 13 ++-------- sdk/python/feast/on_demand_feature_view.py | 24 ++++--------------- .../feast/on_demand_pandas_transformation.py | 2 +- .../on_demand_substrait_transformation.py | 2 +- sdk/python/feast/stream_feature_view.py | 8 +------ 5 files changed, 9 insertions(+), 40 deletions(-) diff --git a/sdk/python/feast/diff/registry_diff.py b/sdk/python/feast/diff/registry_diff.py index 120f5d697a..397c9fff6c 100644 --- a/sdk/python/feast/diff/registry_diff.py +++ b/sdk/python/feast/diff/registry_diff.py @@ -153,17 +153,8 @@ def diff_registry_objects( ) current_spec = cast(OnDemandFeatureViewSpec, current_spec) new_spec = cast(OnDemandFeatureViewSpec, new_spec) - # Check if the old proto is populated and use that if it is - deprecated_udf = current_spec.user_defined_function - feature_transformation_udf = ( - current_spec.feature_transformation.user_defined_function - ) - current_udf = ( - deprecated_udf - if deprecated_udf.body_text != "" - else feature_transformation_udf - ) - new_udf = new_spec.feature_transformation.user_defined_function + current_udf = current_spec.transformation.user_defined_function + new_udf = new_spec.transformation.user_defined_function for _udf_field in current_udf.DESCRIPTOR.fields: if _udf_field.name == "body": continue diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 61e55bb0c0..2cab3b85e7 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -28,10 +28,7 @@ OnDemandSource, ) from feast.protos.feast.core.Transformation_pb2 import ( - FeatureTransformationV2 as FeatureTransformationProto, -) -from feast.protos.feast.core.Transformation_pb2 import ( - UserDefinedFunctionV2 as UserDefinedFunctionProto, + FeatureTransformation as FeatureTransformationProto, ) from feast.type_map import ( feast_value_type_to_pandas_type, @@ -229,7 +226,7 @@ def to_proto(self) -> OnDemandFeatureViewProto: name=self.name, features=[feature.to_proto() for feature in self.features], sources=sources, - feature_transformation=feature_transformation, + transformation=feature_transformation, description=self.description, tags=self.tags, owner=self.owner, @@ -277,7 +274,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): != "" ): transformation = OnDemandPandasTransformation.from_proto( - on_demand_feature_view_proto.spec.feature_transformation.user_defined_function + on_demand_feature_view_proto.spec.transformation.user_defined_function ) elif ( on_demand_feature_view_proto.spec.feature_transformation.WhichOneof( @@ -286,20 +283,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): == "on_demand_substrait_transformation" ): transformation = OnDemandSubstraitTransformation.from_proto( - on_demand_feature_view_proto.spec.feature_transformation.on_demand_substrait_transformation - ) - elif ( - hasattr(on_demand_feature_view_proto.spec, "user_defined_function") - and on_demand_feature_view_proto.spec.feature_transformation.user_defined_function.body_text - == "" - ): - backwards_compatible_udf = UserDefinedFunctionProto( - name=on_demand_feature_view_proto.spec.user_defined_function.name, - body=on_demand_feature_view_proto.spec.user_defined_function.body, - body_text=on_demand_feature_view_proto.spec.user_defined_function.body_text, - ) - transformation = OnDemandPandasTransformation.from_proto( - user_defined_function_proto=backwards_compatible_udf, + on_demand_feature_view_proto.spec.transformation.on_demand_substrait_transformation ) else: raise Exception("At least one transformation type needs to be provided") diff --git a/sdk/python/feast/on_demand_pandas_transformation.py b/sdk/python/feast/on_demand_pandas_transformation.py index 48f5263051..ac995241d1 100644 --- a/sdk/python/feast/on_demand_pandas_transformation.py +++ b/sdk/python/feast/on_demand_pandas_transformation.py @@ -4,7 +4,7 @@ import pandas as pd from feast.protos.feast.core.Transformation_pb2 import ( - UserDefinedFunctionV2 as UserDefinedFunctionProto, + UserDefinedFunction as UserDefinedFunctionProto, ) diff --git a/sdk/python/feast/on_demand_substrait_transformation.py b/sdk/python/feast/on_demand_substrait_transformation.py index 0666739125..5df24dfac7 100644 --- a/sdk/python/feast/on_demand_substrait_transformation.py +++ b/sdk/python/feast/on_demand_substrait_transformation.py @@ -3,7 +3,7 @@ import pyarrow.substrait as substrait # type: ignore # noqa from feast.protos.feast.core.Transformation_pb2 import ( - OnDemandSubstraitTransformationV2 as OnDemandSubstraitTransformationProto, + OnDemandSubstraitTransformation as OnDemandSubstraitTransformationProto, ) diff --git a/sdk/python/feast/stream_feature_view.py b/sdk/python/feast/stream_feature_view.py index e8741a75fe..145262461b 100644 --- a/sdk/python/feast/stream_feature_view.py +++ b/sdk/python/feast/stream_feature_view.py @@ -17,9 +17,6 @@ from feast.field import Field from feast.on_demand_pandas_transformation import OnDemandPandasTransformation from feast.protos.feast.core.DataSource_pb2 import DataSource as DataSourceProto -from feast.protos.feast.core.OnDemandFeatureView_pb2 import ( - UserDefinedFunction as UserDefinedFunctionProto, -) from feast.protos.feast.core.StreamFeatureView_pb2 import ( StreamFeatureView as StreamFeatureViewProto, ) @@ -27,10 +24,7 @@ StreamFeatureViewSpec as StreamFeatureViewSpecProto, ) from feast.protos.feast.core.Transformation_pb2 import ( - FeatureTransformationV2 as FeatureTransformationProto, -) -from feast.protos.feast.core.Transformation_pb2 import ( - UserDefinedFunctionV2 as UserDefinedFunctionProtoV2, + UserDefinedFunction as UserDefinedFunctionProto, ) warnings.simplefilter("once", RuntimeWarning) From ea58ace0c4cf74d566a886cbb2ab1e8fcbf6dd9a Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 17 Mar 2024 22:27:45 -0400 Subject: [PATCH 03/27] updated tests and registry diff function Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/diff/registry_diff.py | 7 +------ sdk/python/feast/on_demand_feature_view.py | 8 ++------ sdk/python/tests/unit/diff/test_registry_diff.py | 4 ++-- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/sdk/python/feast/diff/registry_diff.py b/sdk/python/feast/diff/registry_diff.py index 397c9fff6c..ff6a36ef1f 100644 --- a/sdk/python/feast/diff/registry_diff.py +++ b/sdk/python/feast/diff/registry_diff.py @@ -145,12 +145,7 @@ def diff_registry_objects( if _field.name in FIELDS_TO_IGNORE: continue elif getattr(current_spec, _field.name) != getattr(new_spec, _field.name): - # TODO: Delete "transformation" after we've safely deprecated it from the proto - if _field.name in ["transformation", "feature_transformation"]: - warnings.warn( - "transformation will be deprecated in the future please use feature_transformation instead.", - DeprecationWarning, - ) + if _field.name == "transformation": current_spec = cast(OnDemandFeatureViewSpec, current_spec) new_spec = cast(OnDemandFeatureViewSpec, new_spec) current_udf = current_spec.transformation.user_defined_function diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 2cab3b85e7..4f5155915c 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -266,9 +266,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): ) if ( - on_demand_feature_view_proto.spec.feature_transformation.WhichOneof( - "transformation" - ) + on_demand_feature_view_proto.spec.transformation.WhichOneof("transformation") == "user_defined_function" and on_demand_feature_view_proto.spec.feature_transformation.user_defined_function.body_text != "" @@ -277,9 +275,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): on_demand_feature_view_proto.spec.transformation.user_defined_function ) elif ( - on_demand_feature_view_proto.spec.feature_transformation.WhichOneof( - "transformation" - ) + on_demand_feature_view_proto.spec.transformation.WhichOneof("transformation") == "on_demand_substrait_transformation" ): transformation = OnDemandSubstraitTransformation.from_proto( diff --git a/sdk/python/tests/unit/diff/test_registry_diff.py b/sdk/python/tests/unit/diff/test_registry_diff.py index c209f1e0e0..0f5f5925ce 100644 --- a/sdk/python/tests/unit/diff/test_registry_diff.py +++ b/sdk/python/tests/unit/diff/test_registry_diff.py @@ -140,11 +140,11 @@ def post_changed(inputs: pd.DataFrame) -> pd.DataFrame: # Note we should only now be looking at changes for the feature_transformation field assert ( feast_object_diffs.feast_object_property_diffs[1].property_name - == "feature_transformation.name" + == "transformation.name" ) assert ( feast_object_diffs.feast_object_property_diffs[2].property_name - == "feature_transformation.body_text" + == "transformation.body_text" ) From 1713313f445c6aa63f572a68bb4544d214bf6e86 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 17 Mar 2024 22:48:40 -0400 Subject: [PATCH 04/27] updated base registry Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/infra/registry/base_registry.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/sdk/python/feast/infra/registry/base_registry.py b/sdk/python/feast/infra/registry/base_registry.py index d3d82a80b0..e9092f177a 100644 --- a/sdk/python/feast/infra/registry/base_registry.py +++ b/sdk/python/feast/infra/registry/base_registry.py @@ -663,14 +663,8 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]: key=lambda on_demand_feature_view: on_demand_feature_view.name, ): odfv_dict = self._message_to_sorted_dict(on_demand_feature_view.to_proto()) - # We are logging a warning because the registry object may be read from a proto that is not updated - # i.e., we have to submit dual writes but in order to ensure the read behavior succeeds we have to load - # both objects to compare any changes in the registry - warnings.warn( - "We will be deprecating the usage of spec.userDefinedFunction in a future release please upgrade cautiously.", - DeprecationWarning, - ) - odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"][ + + odfv_dict["spec"]["transformation"]["userDefinedFunction"][ "body" ] = on_demand_feature_view.feature_transformation.udf_string registry_dict["onDemandFeatureViews"].append(odfv_dict) From 4a00c12ae6fd009dde77e30083ec4f7d09bb8b76 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 17 Mar 2024 22:53:30 -0400 Subject: [PATCH 05/27] updated react component Signed-off-by: Francisco Javier Arceo --- ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx b/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx index aac3f6ac5b..4ac23de030 100644 --- a/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx +++ b/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx @@ -57,7 +57,7 @@ const OnDemandFeatureViewOverviewTab = ({ - {data?.spec?.featureTransformation?.userDefinedFunction?.bodyText} + {data?.spec?.transformation?.userDefinedFunction?.bodyText} From 97a8bb64582d35a67af50c97a67a2e82d619be0e Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 17 Mar 2024 22:54:18 -0400 Subject: [PATCH 06/27] formatted Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/on_demand_feature_view.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 4f5155915c..659ea5e75a 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -266,7 +266,9 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): ) if ( - on_demand_feature_view_proto.spec.transformation.WhichOneof("transformation") + on_demand_feature_view_proto.spec.transformation.WhichOneof( + "transformation" + ) == "user_defined_function" and on_demand_feature_view_proto.spec.feature_transformation.user_defined_function.body_text != "" @@ -275,7 +277,9 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): on_demand_feature_view_proto.spec.transformation.user_defined_function ) elif ( - on_demand_feature_view_proto.spec.transformation.WhichOneof("transformation") + on_demand_feature_view_proto.spec.transformation.WhichOneof( + "transformation" + ) == "on_demand_substrait_transformation" ): transformation = OnDemandSubstraitTransformation.from_proto( From 5190d6cf77c90dae3cfbd7be3512fd69ae44465d Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 17 Mar 2024 23:30:15 -0400 Subject: [PATCH 07/27] updated stream feature view proto Signed-off-by: Francisco Javier Arceo --- protos/feast/core/StreamFeatureView.proto | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/protos/feast/core/StreamFeatureView.proto b/protos/feast/core/StreamFeatureView.proto index cb7da0faf3..cbfd8c2d36 100644 --- a/protos/feast/core/StreamFeatureView.proto +++ b/protos/feast/core/StreamFeatureView.proto @@ -81,6 +81,7 @@ message StreamFeatureViewSpec { UserDefinedFunction user_defined_function = 13 [deprecated = true]; + // Mode of execution string mode = 14; @@ -91,6 +92,6 @@ message StreamFeatureViewSpec { string timestamp_field = 16; // Oneof with {user_defined_function, on_demand_substrait_transformation} - FeatureTransformationV2 feature_transformation = 17; + FeatureTransformation transformation = 17; } From 23ae34953f2cf2ffdfb5fb67ab236302fce45fce Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Mon, 18 Mar 2024 06:33:55 -0400 Subject: [PATCH 08/27] making the proto changes backwards compatable Signed-off-by: Francisco Javier Arceo --- protos/feast/core/Transformation.proto | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/protos/feast/core/Transformation.proto b/protos/feast/core/Transformation.proto index 2116ec055d..2a893b0d22 100644 --- a/protos/feast/core/Transformation.proto +++ b/protos/feast/core/Transformation.proto @@ -19,10 +19,12 @@ message UserDefinedFunction { string body_text = 3; } +// A feature transformation executed as a user-defined function message FeatureTransformation { + // Note this Transformation starts at 5 for backwards compatibility oneof transformation { - UserDefinedFunction user_defined_function = 1; - OnDemandSubstraitTransformation on_demand_substrait_transformation = 2; + UserDefinedFunction user_defined_function = 5; + OnDemandSubstraitTransformation on_demand_substrait_transformation = 6; } } From 1d598d234cc72bae36b3238bcbe6153b7738fc1a Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Tue, 19 Mar 2024 20:41:33 -0400 Subject: [PATCH 09/27] trying to make this backwards compatible Signed-off-by: Francisco Javier Arceo --- protos/feast/core/OnDemandFeatureView.proto | 28 +++++++++++++++++-- protos/feast/core/StreamFeatureView.proto | 2 +- protos/feast/core/Transformation.proto | 10 +++---- sdk/python/feast/diff/registry_diff.py | 21 ++++++++++++++ .../feast/infra/registry/base_registry.py | 3 +- sdk/python/feast/on_demand_feature_view.py | 12 ++++---- .../feast/on_demand_pandas_transformation.py | 2 +- .../on_demand_substrait_transformation.py | 2 +- sdk/python/feast/stream_feature_view.py | 8 +++++- .../tests/unit/diff/test_registry_diff.py | 4 +-- sdk/python/tests/unit/test_sql_registry.py | 4 +-- .../OnDemandFeatureViewOverviewTab.tsx | 2 +- 12 files changed, 74 insertions(+), 24 deletions(-) diff --git a/protos/feast/core/OnDemandFeatureView.proto b/protos/feast/core/OnDemandFeatureView.proto index d57465ef96..92c953c16d 100644 --- a/protos/feast/core/OnDemandFeatureView.proto +++ b/protos/feast/core/OnDemandFeatureView.proto @@ -49,8 +49,12 @@ message OnDemandFeatureViewSpec { // Map of sources for this feature view. map sources = 4; + oneof transformation { + UserDefinedFunction user_defined_function = 5; + OnDemandSubstraitTransformation on_demand_substrait_transformation = 9; + } // Oneof with {user_defined_function, on_demand_substrait_transformation} - FeatureTransformation transformation = 5; + FeatureTransformationV2 feature_transformation = 10; // Description of the on demand feature view. string description = 6; @@ -60,7 +64,7 @@ message OnDemandFeatureViewSpec { // Owner of the on demand feature view. string owner = 8; - string mode = 9; + string mode = 11; } message OnDemandFeatureViewMeta { @@ -78,3 +82,23 @@ message OnDemandSource { DataSource request_data_source = 2; } } + +// Serialized representation of python function. +message UserDefinedFunction { + option deprecated = true; + + // The function name + string name = 1; + + // The python-syntax function body (serialized by dill) + bytes body = 2; + + // The string representation of the udf + string body_text = 3; +} + +message OnDemandSubstraitTransformation { + option deprecated = true; + + bytes substrait_plan = 1; +} diff --git a/protos/feast/core/StreamFeatureView.proto b/protos/feast/core/StreamFeatureView.proto index cbfd8c2d36..6ee11f8c7a 100644 --- a/protos/feast/core/StreamFeatureView.proto +++ b/protos/feast/core/StreamFeatureView.proto @@ -92,6 +92,6 @@ message StreamFeatureViewSpec { string timestamp_field = 16; // Oneof with {user_defined_function, on_demand_substrait_transformation} - FeatureTransformation transformation = 17; + FeatureTransformationV2 feature_transformation = 17; } diff --git a/protos/feast/core/Transformation.proto b/protos/feast/core/Transformation.proto index 2a893b0d22..89bbf3daae 100644 --- a/protos/feast/core/Transformation.proto +++ b/protos/feast/core/Transformation.proto @@ -8,7 +8,7 @@ option java_package = "feast.proto.core"; import "google/protobuf/duration.proto"; // Serialized representation of python function. -message UserDefinedFunction { +message UserDefinedFunctionV2 { // The function name string name = 1; @@ -20,14 +20,14 @@ message UserDefinedFunction { } // A feature transformation executed as a user-defined function -message FeatureTransformation { +message FeatureTransformationV2 { // Note this Transformation starts at 5 for backwards compatibility oneof transformation { - UserDefinedFunction user_defined_function = 5; - OnDemandSubstraitTransformation on_demand_substrait_transformation = 6; + UserDefinedFunctionV2 user_defined_function = 5; + OnDemandSubstraitTransformationV2 on_demand_substrait_transformation = 6; } } -message OnDemandSubstraitTransformation { +message OnDemandSubstraitTransformationV2 { bytes substrait_plan = 1; } diff --git a/sdk/python/feast/diff/registry_diff.py b/sdk/python/feast/diff/registry_diff.py index ff6a36ef1f..0c5c30b4b5 100644 --- a/sdk/python/feast/diff/registry_diff.py +++ b/sdk/python/feast/diff/registry_diff.py @@ -164,6 +164,27 @@ def diff_registry_objects( getattr(new_udf, _udf_field.name), ) ) + elif _field.name == "feature_transformation": + current_spec = cast(OnDemandFeatureViewSpec, current_spec) + new_spec = cast(OnDemandFeatureViewSpec, new_spec) + current_udf = ( + current_spec.feature_transformation.user_defined_function + ) + new_udf = new_spec.feature_transformation.user_defined_function + for _udf_field in current_udf.DESCRIPTOR.fields: + if _udf_field.name == "body": + continue + if getattr(current_udf, _udf_field.name) != getattr( + new_udf, _udf_field.name + ): + transition = TransitionType.UPDATE + property_diffs.append( + PropertyDiff( + _field.name + "." + _udf_field.name, + getattr(current_udf, _udf_field.name), + getattr(new_udf, _udf_field.name), + ) + ) else: transition = TransitionType.UPDATE property_diffs.append( diff --git a/sdk/python/feast/infra/registry/base_registry.py b/sdk/python/feast/infra/registry/base_registry.py index e9092f177a..112c7ac35e 100644 --- a/sdk/python/feast/infra/registry/base_registry.py +++ b/sdk/python/feast/infra/registry/base_registry.py @@ -663,8 +663,7 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]: key=lambda on_demand_feature_view: on_demand_feature_view.name, ): odfv_dict = self._message_to_sorted_dict(on_demand_feature_view.to_proto()) - - odfv_dict["spec"]["transformation"]["userDefinedFunction"][ + odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"][ "body" ] = on_demand_feature_view.feature_transformation.udf_string registry_dict["onDemandFeatureViews"].append(odfv_dict) diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 659ea5e75a..958d91aeb1 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -28,7 +28,7 @@ OnDemandSource, ) from feast.protos.feast.core.Transformation_pb2 import ( - FeatureTransformation as FeatureTransformationProto, + FeatureTransformationV2 as FeatureTransformationProto, ) from feast.type_map import ( feast_value_type_to_pandas_type, @@ -226,7 +226,7 @@ def to_proto(self) -> OnDemandFeatureViewProto: name=self.name, features=[feature.to_proto() for feature in self.features], sources=sources, - transformation=feature_transformation, + feature_transformation=feature_transformation, description=self.description, tags=self.tags, owner=self.owner, @@ -266,7 +266,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): ) if ( - on_demand_feature_view_proto.spec.transformation.WhichOneof( + on_demand_feature_view_proto.spec.feature_transformation.WhichOneof( "transformation" ) == "user_defined_function" @@ -274,16 +274,16 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): != "" ): transformation = OnDemandPandasTransformation.from_proto( - on_demand_feature_view_proto.spec.transformation.user_defined_function + on_demand_feature_view_proto.spec.feature_transformation.user_defined_function ) elif ( - on_demand_feature_view_proto.spec.transformation.WhichOneof( + on_demand_feature_view_proto.spec.feature_transformation.WhichOneof( "transformation" ) == "on_demand_substrait_transformation" ): transformation = OnDemandSubstraitTransformation.from_proto( - on_demand_feature_view_proto.spec.transformation.on_demand_substrait_transformation + on_demand_feature_view_proto.spec.feature_transformation.on_demand_substrait_transformation ) else: raise Exception("At least one transformation type needs to be provided") diff --git a/sdk/python/feast/on_demand_pandas_transformation.py b/sdk/python/feast/on_demand_pandas_transformation.py index ac995241d1..48f5263051 100644 --- a/sdk/python/feast/on_demand_pandas_transformation.py +++ b/sdk/python/feast/on_demand_pandas_transformation.py @@ -4,7 +4,7 @@ import pandas as pd from feast.protos.feast.core.Transformation_pb2 import ( - UserDefinedFunction as UserDefinedFunctionProto, + UserDefinedFunctionV2 as UserDefinedFunctionProto, ) diff --git a/sdk/python/feast/on_demand_substrait_transformation.py b/sdk/python/feast/on_demand_substrait_transformation.py index 5df24dfac7..0666739125 100644 --- a/sdk/python/feast/on_demand_substrait_transformation.py +++ b/sdk/python/feast/on_demand_substrait_transformation.py @@ -3,7 +3,7 @@ import pyarrow.substrait as substrait # type: ignore # noqa from feast.protos.feast.core.Transformation_pb2 import ( - OnDemandSubstraitTransformation as OnDemandSubstraitTransformationProto, + OnDemandSubstraitTransformationV2 as OnDemandSubstraitTransformationProto, ) diff --git a/sdk/python/feast/stream_feature_view.py b/sdk/python/feast/stream_feature_view.py index 145262461b..e8741a75fe 100644 --- a/sdk/python/feast/stream_feature_view.py +++ b/sdk/python/feast/stream_feature_view.py @@ -17,6 +17,9 @@ from feast.field import Field from feast.on_demand_pandas_transformation import OnDemandPandasTransformation from feast.protos.feast.core.DataSource_pb2 import DataSource as DataSourceProto +from feast.protos.feast.core.OnDemandFeatureView_pb2 import ( + UserDefinedFunction as UserDefinedFunctionProto, +) from feast.protos.feast.core.StreamFeatureView_pb2 import ( StreamFeatureView as StreamFeatureViewProto, ) @@ -24,7 +27,10 @@ StreamFeatureViewSpec as StreamFeatureViewSpecProto, ) from feast.protos.feast.core.Transformation_pb2 import ( - UserDefinedFunction as UserDefinedFunctionProto, + FeatureTransformationV2 as FeatureTransformationProto, +) +from feast.protos.feast.core.Transformation_pb2 import ( + UserDefinedFunctionV2 as UserDefinedFunctionProtoV2, ) warnings.simplefilter("once", RuntimeWarning) diff --git a/sdk/python/tests/unit/diff/test_registry_diff.py b/sdk/python/tests/unit/diff/test_registry_diff.py index 0f5f5925ce..c209f1e0e0 100644 --- a/sdk/python/tests/unit/diff/test_registry_diff.py +++ b/sdk/python/tests/unit/diff/test_registry_diff.py @@ -140,11 +140,11 @@ def post_changed(inputs: pd.DataFrame) -> pd.DataFrame: # Note we should only now be looking at changes for the feature_transformation field assert ( feast_object_diffs.feast_object_property_diffs[1].property_name - == "transformation.name" + == "feature_transformation.name" ) assert ( feast_object_diffs.feast_object_property_diffs[2].property_name - == "transformation.body_text" + == "feature_transformation.body_text" ) diff --git a/sdk/python/tests/unit/test_sql_registry.py b/sdk/python/tests/unit/test_sql_registry.py index 094b8967c1..90c9f9e75c 100644 --- a/sdk/python/tests/unit/test_sql_registry.py +++ b/sdk/python/tests/unit/test_sql_registry.py @@ -382,8 +382,8 @@ def location_features_from_push(inputs: pd.DataFrame) -> pd.DataFrame: @pytest.mark.parametrize( "sql_registry", [ - lazy_fixture("mysql_registry"), - lazy_fixture("pg_registry"), + # lazy_fixture("mysql_registry"), + # lazy_fixture("pg_registry"), lazy_fixture("sqlite_registry"), ], ) diff --git a/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx b/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx index 4ac23de030..dc34528e7e 100644 --- a/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx +++ b/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx @@ -57,7 +57,7 @@ const OnDemandFeatureViewOverviewTab = ({ - {data?.spec?.transformation?.userDefinedFunction?.bodyText} + {data?.spec?.feature_transformation?.userDefinedFunction?.bodyText} From 81c6f82e42d149c40a2f299f73d78a9e1fc2c68d Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Tue, 19 Mar 2024 20:56:45 -0400 Subject: [PATCH 10/27] caught a bug and fixed the linter Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/diff/registry_diff.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/diff/registry_diff.py b/sdk/python/feast/diff/registry_diff.py index 0c5c30b4b5..f140e620a4 100644 --- a/sdk/python/feast/diff/registry_diff.py +++ b/sdk/python/feast/diff/registry_diff.py @@ -148,8 +148,8 @@ def diff_registry_objects( if _field.name == "transformation": current_spec = cast(OnDemandFeatureViewSpec, current_spec) new_spec = cast(OnDemandFeatureViewSpec, new_spec) - current_udf = current_spec.transformation.user_defined_function - new_udf = new_spec.transformation.user_defined_function + current_udf = current_spec.feature_transformation.user_defined_function + new_udf = new_spec.feature_transformation.user_defined_function for _udf_field in current_udf.DESCRIPTOR.fields: if _udf_field.name == "body": continue From 7687e23bf7fc9341f77c74c7e3bd5a81c8f76e4f Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Tue, 19 Mar 2024 21:02:54 -0400 Subject: [PATCH 11/27] actually linted Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/diff/registry_diff.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/diff/registry_diff.py b/sdk/python/feast/diff/registry_diff.py index f140e620a4..b72dbbed08 100644 --- a/sdk/python/feast/diff/registry_diff.py +++ b/sdk/python/feast/diff/registry_diff.py @@ -148,7 +148,9 @@ def diff_registry_objects( if _field.name == "transformation": current_spec = cast(OnDemandFeatureViewSpec, current_spec) new_spec = cast(OnDemandFeatureViewSpec, new_spec) - current_udf = current_spec.feature_transformation.user_defined_function + current_udf = ( + current_spec.feature_transformation.user_defined_function + ) new_udf = new_spec.feature_transformation.user_defined_function for _udf_field in current_udf.DESCRIPTOR.fields: if _udf_field.name == "body": From 65078084eda14787856c9260a8fbaf87baa9907c Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Tue, 19 Mar 2024 21:57:38 -0400 Subject: [PATCH 12/27] updated ui component Signed-off-by: Francisco Javier Arceo --- ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx b/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx index dc34528e7e..aac3f6ac5b 100644 --- a/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx +++ b/ui/src/pages/feature-views/OnDemandFeatureViewOverviewTab.tsx @@ -57,7 +57,7 @@ const OnDemandFeatureViewOverviewTab = ({ - {data?.spec?.feature_transformation?.userDefinedFunction?.bodyText} + {data?.spec?.featureTransformation?.userDefinedFunction?.bodyText} From f44c2274d3bb0dc0f53250c838f197b1e433c84b Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Wed, 20 Mar 2024 09:52:00 -0400 Subject: [PATCH 13/27] accidentally commented out fixtures Signed-off-by: Francisco Javier Arceo --- sdk/python/tests/unit/test_sql_registry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/unit/test_sql_registry.py b/sdk/python/tests/unit/test_sql_registry.py index 90c9f9e75c..094b8967c1 100644 --- a/sdk/python/tests/unit/test_sql_registry.py +++ b/sdk/python/tests/unit/test_sql_registry.py @@ -382,8 +382,8 @@ def location_features_from_push(inputs: pd.DataFrame) -> pd.DataFrame: @pytest.mark.parametrize( "sql_registry", [ - # lazy_fixture("mysql_registry"), - # lazy_fixture("pg_registry"), + lazy_fixture("mysql_registry"), + lazy_fixture("pg_registry"), lazy_fixture("sqlite_registry"), ], ) From dd2a5ca5fc6d35308586fa2ead112be5e4f2fa8c Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Fri, 22 Mar 2024 11:19:34 -0400 Subject: [PATCH 14/27] Updated Signed-off-by: Francisco Javier Arceo --- protos/feast/core/OnDemandFeatureView.proto | 4 +- protos/feast/core/StreamFeatureView.proto | 1 - sdk/python/feast/diff/registry_diff.py | 33 ++++------ .../feast/infra/registry/base_registry.py | 7 +++ .../tests/unit/test_on_demand_feature_view.py | 62 ------------------- 5 files changed, 21 insertions(+), 86 deletions(-) diff --git a/protos/feast/core/OnDemandFeatureView.proto b/protos/feast/core/OnDemandFeatureView.proto index 92c953c16d..cd3ceba150 100644 --- a/protos/feast/core/OnDemandFeatureView.proto +++ b/protos/feast/core/OnDemandFeatureView.proto @@ -50,8 +50,8 @@ message OnDemandFeatureViewSpec { map sources = 4; oneof transformation { - UserDefinedFunction user_defined_function = 5; - OnDemandSubstraitTransformation on_demand_substrait_transformation = 9; + UserDefinedFunction user_defined_function = 5 [deprecated = true]; + OnDemandSubstraitTransformation on_demand_substrait_transformation = 9 [deprecated = true]; } // Oneof with {user_defined_function, on_demand_substrait_transformation} FeatureTransformationV2 feature_transformation = 10; diff --git a/protos/feast/core/StreamFeatureView.proto b/protos/feast/core/StreamFeatureView.proto index 6ee11f8c7a..cb7da0faf3 100644 --- a/protos/feast/core/StreamFeatureView.proto +++ b/protos/feast/core/StreamFeatureView.proto @@ -81,7 +81,6 @@ message StreamFeatureViewSpec { UserDefinedFunction user_defined_function = 13 [deprecated = true]; - // Mode of execution string mode = 14; diff --git a/sdk/python/feast/diff/registry_diff.py b/sdk/python/feast/diff/registry_diff.py index b72dbbed08..120f5d697a 100644 --- a/sdk/python/feast/diff/registry_diff.py +++ b/sdk/python/feast/diff/registry_diff.py @@ -145,32 +145,23 @@ def diff_registry_objects( if _field.name in FIELDS_TO_IGNORE: continue elif getattr(current_spec, _field.name) != getattr(new_spec, _field.name): - if _field.name == "transformation": + # TODO: Delete "transformation" after we've safely deprecated it from the proto + if _field.name in ["transformation", "feature_transformation"]: + warnings.warn( + "transformation will be deprecated in the future please use feature_transformation instead.", + DeprecationWarning, + ) current_spec = cast(OnDemandFeatureViewSpec, current_spec) new_spec = cast(OnDemandFeatureViewSpec, new_spec) - current_udf = ( + # Check if the old proto is populated and use that if it is + deprecated_udf = current_spec.user_defined_function + feature_transformation_udf = ( current_spec.feature_transformation.user_defined_function ) - new_udf = new_spec.feature_transformation.user_defined_function - for _udf_field in current_udf.DESCRIPTOR.fields: - if _udf_field.name == "body": - continue - if getattr(current_udf, _udf_field.name) != getattr( - new_udf, _udf_field.name - ): - transition = TransitionType.UPDATE - property_diffs.append( - PropertyDiff( - _field.name + "." + _udf_field.name, - getattr(current_udf, _udf_field.name), - getattr(new_udf, _udf_field.name), - ) - ) - elif _field.name == "feature_transformation": - current_spec = cast(OnDemandFeatureViewSpec, current_spec) - new_spec = cast(OnDemandFeatureViewSpec, new_spec) current_udf = ( - current_spec.feature_transformation.user_defined_function + deprecated_udf + if deprecated_udf.body_text != "" + else feature_transformation_udf ) new_udf = new_spec.feature_transformation.user_defined_function for _udf_field in current_udf.DESCRIPTOR.fields: diff --git a/sdk/python/feast/infra/registry/base_registry.py b/sdk/python/feast/infra/registry/base_registry.py index 112c7ac35e..d3d82a80b0 100644 --- a/sdk/python/feast/infra/registry/base_registry.py +++ b/sdk/python/feast/infra/registry/base_registry.py @@ -663,6 +663,13 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]: key=lambda on_demand_feature_view: on_demand_feature_view.name, ): odfv_dict = self._message_to_sorted_dict(on_demand_feature_view.to_proto()) + # We are logging a warning because the registry object may be read from a proto that is not updated + # i.e., we have to submit dual writes but in order to ensure the read behavior succeeds we have to load + # both objects to compare any changes in the registry + warnings.warn( + "We will be deprecating the usage of spec.userDefinedFunction in a future release please upgrade cautiously.", + DeprecationWarning, + ) odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"][ "body" ] = on_demand_feature_view.feature_transformation.udf_string diff --git a/sdk/python/tests/unit/test_on_demand_feature_view.py b/sdk/python/tests/unit/test_on_demand_feature_view.py index b83449519f..21c07b9d7f 100644 --- a/sdk/python/tests/unit/test_on_demand_feature_view.py +++ b/sdk/python/tests/unit/test_on_demand_feature_view.py @@ -133,65 +133,3 @@ def test_hash(): on_demand_feature_view_5.feature_transformation == on_demand_feature_view_5.transformation ) - - -@pytest.mark.filterwarnings("ignore:udf and udf_string parameters are deprecated") -def test_from_proto_backwards_compatable_udf(): - file_source = FileSource(name="my-file-source", path="test.parquet") - feature_view = FeatureView( - name="my-feature-view", - entities=[], - schema=[ - Field(name="feature1", dtype=Float32), - Field(name="feature2", dtype=Float32), - ], - source=file_source, - ) - sources = [feature_view] - on_demand_feature_view = OnDemandFeatureView( - name="my-on-demand-feature-view", - sources=sources, - schema=[ - Field(name="output1", dtype=Float32), - Field(name="output2", dtype=Float32), - ], - transformation=OnDemandPandasTransformation( - udf=udf1, udf_string="udf1 source code" - ), - ) - - # We need a proto with the "udf1 source code" in the user_defined_function.body_text - # and to populate it in feature_transformation - proto = on_demand_feature_view.to_proto() - assert ( - on_demand_feature_view.transformation.udf_string - == proto.spec.feature_transformation.user_defined_function.body_text - ) - # Because of the current set of code this is just confirming it is empty - assert proto.spec.user_defined_function.body_text == "" - assert proto.spec.user_defined_function.body == b"" - assert proto.spec.user_defined_function.name == "" - - # Assuming we pull it from the registry we set it to the feature_transformation proto values - proto.spec.user_defined_function.name = ( - proto.spec.feature_transformation.user_defined_function.name - ) - proto.spec.user_defined_function.body = ( - proto.spec.feature_transformation.user_defined_function.body - ) - proto.spec.user_defined_function.body_text = ( - proto.spec.feature_transformation.user_defined_function.body_text - ) - - # And now we're going to null the feature_transformation proto object before reserializing the entire proto - # proto.spec.user_defined_function.body_text = on_demand_feature_view.transformation.udf_string - proto.spec.feature_transformation.user_defined_function.name = "" - proto.spec.feature_transformation.user_defined_function.body = b"" - proto.spec.feature_transformation.user_defined_function.body_text = "" - - # And now we expect the to get the same object back under feature_transformation - reserialized_proto = OnDemandFeatureView.from_proto(proto) - assert ( - reserialized_proto.feature_transformation.udf_string - == on_demand_feature_view.feature_transformation.udf_string - ) From 9ac6793116745d35ae51e18b05b8eb7afc5b5521 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Fri, 22 Mar 2024 11:59:19 -0400 Subject: [PATCH 15/27] incrementing protos Signed-off-by: Francisco Javier Arceo --- protos/feast/core/Transformation.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protos/feast/core/Transformation.proto b/protos/feast/core/Transformation.proto index 89bbf3daae..cde2833fa4 100644 --- a/protos/feast/core/Transformation.proto +++ b/protos/feast/core/Transformation.proto @@ -23,8 +23,8 @@ message UserDefinedFunctionV2 { message FeatureTransformationV2 { // Note this Transformation starts at 5 for backwards compatibility oneof transformation { - UserDefinedFunctionV2 user_defined_function = 5; - OnDemandSubstraitTransformationV2 on_demand_substrait_transformation = 6; + UserDefinedFunctionV2 user_defined_function = 1; + OnDemandSubstraitTransformationV2 on_demand_substrait_transformation = 2; } } From 5a1db096e95b9cee533147f78e693a36ec253ffb Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Fri, 22 Mar 2024 15:03:14 -0400 Subject: [PATCH 16/27] updated tests Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/on_demand_feature_view.py | 8 +++ .../tests/unit/test_on_demand_feature_view.py | 62 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 958d91aeb1..f776164985 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -285,6 +285,14 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): transformation = OnDemandSubstraitTransformation.from_proto( on_demand_feature_view_proto.spec.feature_transformation.on_demand_substrait_transformation ) + elif ( + hasattr(on_demand_feature_view_proto.spec, "user_defined_function") + and on_demand_feature_view_proto.spec.feature_transformation.user_defined_function.body_text + == "" + ): + transformation = OnDemandPandasTransformation.from_proto( + on_demand_feature_view_proto.spec.user_defined_function + ) else: raise Exception("At least one transformation type needs to be provided") diff --git a/sdk/python/tests/unit/test_on_demand_feature_view.py b/sdk/python/tests/unit/test_on_demand_feature_view.py index 21c07b9d7f..b83449519f 100644 --- a/sdk/python/tests/unit/test_on_demand_feature_view.py +++ b/sdk/python/tests/unit/test_on_demand_feature_view.py @@ -133,3 +133,65 @@ def test_hash(): on_demand_feature_view_5.feature_transformation == on_demand_feature_view_5.transformation ) + + +@pytest.mark.filterwarnings("ignore:udf and udf_string parameters are deprecated") +def test_from_proto_backwards_compatable_udf(): + file_source = FileSource(name="my-file-source", path="test.parquet") + feature_view = FeatureView( + name="my-feature-view", + entities=[], + schema=[ + Field(name="feature1", dtype=Float32), + Field(name="feature2", dtype=Float32), + ], + source=file_source, + ) + sources = [feature_view] + on_demand_feature_view = OnDemandFeatureView( + name="my-on-demand-feature-view", + sources=sources, + schema=[ + Field(name="output1", dtype=Float32), + Field(name="output2", dtype=Float32), + ], + transformation=OnDemandPandasTransformation( + udf=udf1, udf_string="udf1 source code" + ), + ) + + # We need a proto with the "udf1 source code" in the user_defined_function.body_text + # and to populate it in feature_transformation + proto = on_demand_feature_view.to_proto() + assert ( + on_demand_feature_view.transformation.udf_string + == proto.spec.feature_transformation.user_defined_function.body_text + ) + # Because of the current set of code this is just confirming it is empty + assert proto.spec.user_defined_function.body_text == "" + assert proto.spec.user_defined_function.body == b"" + assert proto.spec.user_defined_function.name == "" + + # Assuming we pull it from the registry we set it to the feature_transformation proto values + proto.spec.user_defined_function.name = ( + proto.spec.feature_transformation.user_defined_function.name + ) + proto.spec.user_defined_function.body = ( + proto.spec.feature_transformation.user_defined_function.body + ) + proto.spec.user_defined_function.body_text = ( + proto.spec.feature_transformation.user_defined_function.body_text + ) + + # And now we're going to null the feature_transformation proto object before reserializing the entire proto + # proto.spec.user_defined_function.body_text = on_demand_feature_view.transformation.udf_string + proto.spec.feature_transformation.user_defined_function.name = "" + proto.spec.feature_transformation.user_defined_function.body = b"" + proto.spec.feature_transformation.user_defined_function.body_text = "" + + # And now we expect the to get the same object back under feature_transformation + reserialized_proto = OnDemandFeatureView.from_proto(proto) + assert ( + reserialized_proto.feature_transformation.udf_string + == on_demand_feature_view.feature_transformation.udf_string + ) From e6bf1e950da9c4301be293b78ba9a99837db467c Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Fri, 22 Mar 2024 16:59:04 -0400 Subject: [PATCH 17/27] fixed linting issue and made backwards compatible Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/on_demand_feature_view.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index f776164985..61e55bb0c0 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -30,6 +30,9 @@ from feast.protos.feast.core.Transformation_pb2 import ( FeatureTransformationV2 as FeatureTransformationProto, ) +from feast.protos.feast.core.Transformation_pb2 import ( + UserDefinedFunctionV2 as UserDefinedFunctionProto, +) from feast.type_map import ( feast_value_type_to_pandas_type, python_type_to_feast_value_type, @@ -290,8 +293,13 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): and on_demand_feature_view_proto.spec.feature_transformation.user_defined_function.body_text == "" ): + backwards_compatible_udf = UserDefinedFunctionProto( + name=on_demand_feature_view_proto.spec.user_defined_function.name, + body=on_demand_feature_view_proto.spec.user_defined_function.body, + body_text=on_demand_feature_view_proto.spec.user_defined_function.body_text, + ) transformation = OnDemandPandasTransformation.from_proto( - on_demand_feature_view_proto.spec.user_defined_function + user_defined_function_proto=backwards_compatible_udf, ) else: raise Exception("At least one transformation type needs to be provided") From 0daf0273f73dae11fee53a88a2cf62a0e509636a Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 24 Mar 2024 12:44:38 -0400 Subject: [PATCH 18/27] feat: Renaming OnDemandTransformations to Transformations Signed-off-by: Francisco Javier Arceo --- sdk/python/feast/on_demand_feature_view.py | 28 +++++++++---------- sdk/python/feast/stream_feature_view.py | 10 +++---- sdk/python/feast/transformation/__init__.py | 0 .../pandas_transformation.py} | 6 ++-- .../substrait_transformation.py} | 6 ++-- .../feature_repos/universal/feature_views.py | 6 ++-- .../tests/unit/test_on_demand_feature_view.py | 27 +++++------------- 7 files changed, 34 insertions(+), 49 deletions(-) create mode 100644 sdk/python/feast/transformation/__init__.py rename sdk/python/feast/{on_demand_pandas_transformation.py => transformation/pandas_transformation.py} (91%) rename sdk/python/feast/{on_demand_substrait_transformation.py => transformation/substrait_transformation.py} (90%) diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 61e55bb0c0..9ca76a2a72 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -17,8 +17,6 @@ from feast.feature_view import FeatureView from feast.feature_view_projection import FeatureViewProjection from feast.field import Field, from_value_type -from feast.on_demand_pandas_transformation import OnDemandPandasTransformation -from feast.on_demand_substrait_transformation import OnDemandSubstraitTransformation from feast.protos.feast.core.OnDemandFeatureView_pb2 import ( OnDemandFeatureView as OnDemandFeatureViewProto, ) @@ -33,6 +31,8 @@ from feast.protos.feast.core.Transformation_pb2 import ( UserDefinedFunctionV2 as UserDefinedFunctionProto, ) +from feast.transformation.pandas_transformation import PandasTransformation +from feast.transformation.substrait_transformation import SubstraitTransformation from feast.type_map import ( feast_value_type_to_pandas_type, python_type_to_feast_value_type, @@ -68,8 +68,8 @@ class OnDemandFeatureView(BaseFeatureView): features: List[Field] source_feature_view_projections: Dict[str, FeatureViewProjection] source_request_sources: Dict[str, RequestSource] - transformation: Union[OnDemandPandasTransformation] - feature_transformation: Union[OnDemandPandasTransformation] + transformation: Union[PandasTransformation] + feature_transformation: Union[PandasTransformation] description: str tags: Dict[str, str] owner: str @@ -89,8 +89,8 @@ def __init__( # noqa: C901 ], udf: Optional[FunctionType] = None, udf_string: str = "", - transformation: Optional[Union[OnDemandPandasTransformation]] = None, - feature_transformation: Optional[Union[OnDemandPandasTransformation]] = None, + transformation: Optional[Union[PandasTransformation]] = None, + feature_transformation: Optional[Union[PandasTransformation]] = None, description: str = "", tags: Optional[Dict[str, str]] = None, owner: str = "", @@ -129,7 +129,7 @@ def __init__( # noqa: C901 "udf and udf_string parameters are deprecated. Please use transformation=OnDemandPandasTransformation(udf, udf_string) instead.", DeprecationWarning, ) - transformation = OnDemandPandasTransformation(udf, udf_string) + transformation = PandasTransformation(udf, udf_string) else: raise Exception( "OnDemandFeatureView needs to be initialized with either transformation or udf arguments" @@ -219,10 +219,10 @@ def to_proto(self) -> OnDemandFeatureViewProto: feature_transformation = FeatureTransformationProto( user_defined_function=self.transformation.to_proto() - if type(self.transformation) == OnDemandPandasTransformation + if type(self.transformation) == PandasTransformation else None, on_demand_substrait_transformation=self.transformation.to_proto() - if type(self.transformation) == OnDemandSubstraitTransformation + if type(self.transformation) == SubstraitTransformation else None, # type: ignore ) spec = OnDemandFeatureViewSpec( @@ -276,7 +276,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): and on_demand_feature_view_proto.spec.feature_transformation.user_defined_function.body_text != "" ): - transformation = OnDemandPandasTransformation.from_proto( + transformation = PandasTransformation.from_proto( on_demand_feature_view_proto.spec.feature_transformation.user_defined_function ) elif ( @@ -285,7 +285,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): ) == "on_demand_substrait_transformation" ): - transformation = OnDemandSubstraitTransformation.from_proto( + transformation = SubstraitTransformation.from_proto( on_demand_feature_view_proto.spec.feature_transformation.on_demand_substrait_transformation ) elif ( @@ -298,7 +298,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): body=on_demand_feature_view_proto.spec.user_defined_function.body, body_text=on_demand_feature_view_proto.spec.user_defined_function.body_text, ) - transformation = OnDemandPandasTransformation.from_proto( + transformation = PandasTransformation.from_proto( user_defined_function_proto=backwards_compatible_udf, ) else: @@ -540,13 +540,13 @@ def decorator(user_function): expr = user_function(ibis.table(input_fields, "t")) - transformation = OnDemandSubstraitTransformation( + transformation = SubstraitTransformation( substrait_plan=compiler.compile(expr).SerializeToString() ) else: udf_string = dill.source.getsource(user_function) mainify(user_function) - transformation = OnDemandPandasTransformation(user_function, udf_string) + transformation = PandasTransformation(user_function, udf_string) on_demand_feature_view_obj = OnDemandFeatureView( name=user_function.__name__, diff --git a/sdk/python/feast/stream_feature_view.py b/sdk/python/feast/stream_feature_view.py index e8741a75fe..0d1125d2bd 100644 --- a/sdk/python/feast/stream_feature_view.py +++ b/sdk/python/feast/stream_feature_view.py @@ -15,7 +15,6 @@ from feast.entity import Entity from feast.feature_view import FeatureView from feast.field import Field -from feast.on_demand_pandas_transformation import OnDemandPandasTransformation from feast.protos.feast.core.DataSource_pb2 import DataSource as DataSourceProto from feast.protos.feast.core.OnDemandFeatureView_pb2 import ( UserDefinedFunction as UserDefinedFunctionProto, @@ -32,6 +31,7 @@ from feast.protos.feast.core.Transformation_pb2 import ( UserDefinedFunctionV2 as UserDefinedFunctionProtoV2, ) +from feast.transformation.pandas_transformation import PandasTransformation warnings.simplefilter("once", RuntimeWarning) @@ -80,7 +80,7 @@ class StreamFeatureView(FeatureView): materialization_intervals: List[Tuple[datetime, datetime]] udf: Optional[FunctionType] udf_string: Optional[str] - feature_transformation: Optional[OnDemandPandasTransformation] + feature_transformation: Optional[PandasTransformation] def __init__( self, @@ -99,7 +99,7 @@ def __init__( timestamp_field: Optional[str] = "", udf: Optional[FunctionType] = None, udf_string: Optional[str] = "", - feature_transformation: Optional[Union[OnDemandPandasTransformation]] = None, + feature_transformation: Optional[Union[PandasTransformation]] = None, ): if not flags_helper.is_test(): warnings.warn( @@ -371,9 +371,7 @@ def decorator(user_function): schema=schema, udf=user_function, udf_string=udf_string, - feature_transformation=OnDemandPandasTransformation( - user_function, udf_string - ), + feature_transformation=PandasTransformation(user_function, udf_string), description=description, tags=tags, online=online, diff --git a/sdk/python/feast/transformation/__init__.py b/sdk/python/feast/transformation/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sdk/python/feast/on_demand_pandas_transformation.py b/sdk/python/feast/transformation/pandas_transformation.py similarity index 91% rename from sdk/python/feast/on_demand_pandas_transformation.py rename to sdk/python/feast/transformation/pandas_transformation.py index 48f5263051..c6b3eed1b3 100644 --- a/sdk/python/feast/on_demand_pandas_transformation.py +++ b/sdk/python/feast/transformation/pandas_transformation.py @@ -8,7 +8,7 @@ ) -class OnDemandPandasTransformation: +class PandasTransformation: def __init__(self, udf: FunctionType, udf_string: str = ""): """ Creates an OnDemandPandasTransformation object. @@ -25,7 +25,7 @@ def transform(self, df: pd.DataFrame) -> pd.DataFrame: return self.udf.__call__(df) def __eq__(self, other): - if not isinstance(other, OnDemandPandasTransformation): + if not isinstance(other, PandasTransformation): raise TypeError( "Comparisons should only involve OnDemandPandasTransformation class objects." ) @@ -47,7 +47,7 @@ def to_proto(self) -> UserDefinedFunctionProto: @classmethod def from_proto(cls, user_defined_function_proto: UserDefinedFunctionProto): - return OnDemandPandasTransformation( + return PandasTransformation( udf=dill.loads(user_defined_function_proto.body), udf_string=user_defined_function_proto.body_text, ) diff --git a/sdk/python/feast/on_demand_substrait_transformation.py b/sdk/python/feast/transformation/substrait_transformation.py similarity index 90% rename from sdk/python/feast/on_demand_substrait_transformation.py rename to sdk/python/feast/transformation/substrait_transformation.py index 0666739125..4b96541c8a 100644 --- a/sdk/python/feast/on_demand_substrait_transformation.py +++ b/sdk/python/feast/transformation/substrait_transformation.py @@ -7,7 +7,7 @@ ) -class OnDemandSubstraitTransformation: +class SubstraitTransformation: def __init__(self, substrait_plan: bytes): """ Creates an OnDemandSubstraitTransformation object. @@ -27,7 +27,7 @@ def table_provider(names, schema: pyarrow.Schema): return table.to_pandas() def __eq__(self, other): - if not isinstance(other, OnDemandSubstraitTransformation): + if not isinstance(other, SubstraitTransformation): raise TypeError( "Comparisons should only involve OnDemandSubstraitTransformation class objects." ) @@ -45,6 +45,6 @@ def from_proto( cls, on_demand_substrait_transformation_proto: OnDemandSubstraitTransformationProto, ): - return OnDemandSubstraitTransformation( + return SubstraitTransformation( substrait_plan=on_demand_substrait_transformation_proto.substrait_plan ) diff --git a/sdk/python/tests/integration/feature_repos/universal/feature_views.py b/sdk/python/tests/integration/feature_repos/universal/feature_views.py index 421ef41601..6ab01901da 100644 --- a/sdk/python/tests/integration/feature_repos/universal/feature_views.py +++ b/sdk/python/tests/integration/feature_repos/universal/feature_views.py @@ -15,7 +15,7 @@ ) from feast.data_source import DataSource, RequestSource from feast.feature_view_projection import FeatureViewProjection -from feast.on_demand_feature_view import OnDemandPandasTransformation +from feast.on_demand_feature_view import PandasTransformation from feast.types import Array, FeastType, Float32, Float64, Int32, Int64 from tests.integration.feature_repos.universal.entities import ( customer, @@ -71,7 +71,7 @@ def conv_rate_plus_100_feature_view( name=conv_rate_plus_100.__name__, schema=[] if infer_features else _features, sources=sources, - transformation=OnDemandPandasTransformation( + transformation=PandasTransformation( udf=conv_rate_plus_100, udf_string="raw udf source" ), ) @@ -110,7 +110,7 @@ def similarity_feature_view( name=similarity.__name__, sources=sources, schema=[] if infer_features else _fields, - transformation=OnDemandPandasTransformation( + transformation=PandasTransformation( udf=similarity, udf_string="similarity raw udf" ), ) diff --git a/sdk/python/tests/unit/test_on_demand_feature_view.py b/sdk/python/tests/unit/test_on_demand_feature_view.py index b83449519f..8cea3be352 100644 --- a/sdk/python/tests/unit/test_on_demand_feature_view.py +++ b/sdk/python/tests/unit/test_on_demand_feature_view.py @@ -18,10 +18,7 @@ from feast.feature_view import FeatureView from feast.field import Field from feast.infra.offline_stores.file_source import FileSource -from feast.on_demand_feature_view import ( - OnDemandFeatureView, - OnDemandPandasTransformation, -) +from feast.on_demand_feature_view import OnDemandFeatureView, PandasTransformation from feast.types import Float32 @@ -59,9 +56,7 @@ def test_hash(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=OnDemandPandasTransformation( - udf=udf1, udf_string="udf1 source code" - ), + transformation=PandasTransformation(udf=udf1, udf_string="udf1 source code"), ) on_demand_feature_view_2 = OnDemandFeatureView( name="my-on-demand-feature-view", @@ -70,9 +65,7 @@ def test_hash(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=OnDemandPandasTransformation( - udf=udf1, udf_string="udf1 source code" - ), + transformation=PandasTransformation(udf=udf1, udf_string="udf1 source code"), ) on_demand_feature_view_3 = OnDemandFeatureView( name="my-on-demand-feature-view", @@ -81,9 +74,7 @@ def test_hash(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=OnDemandPandasTransformation( - udf=udf2, udf_string="udf2 source code" - ), + transformation=PandasTransformation(udf=udf2, udf_string="udf2 source code"), ) on_demand_feature_view_4 = OnDemandFeatureView( name="my-on-demand-feature-view", @@ -92,9 +83,7 @@ def test_hash(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=OnDemandPandasTransformation( - udf=udf2, udf_string="udf2 source code" - ), + transformation=PandasTransformation(udf=udf2, udf_string="udf2 source code"), description="test", ) on_demand_feature_view_5 = OnDemandFeatureView( @@ -126,7 +115,7 @@ def test_hash(): } assert len(s4) == 3 - assert on_demand_feature_view_5.transformation == OnDemandPandasTransformation( + assert on_demand_feature_view_5.transformation == PandasTransformation( udf2, "udf2 source code" ) assert ( @@ -155,9 +144,7 @@ def test_from_proto_backwards_compatable_udf(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=OnDemandPandasTransformation( - udf=udf1, udf_string="udf1 source code" - ), + transformation=PandasTransformation(udf=udf1, udf_string="udf1 source code"), ) # We need a proto with the "udf1 source code" in the user_defined_function.body_text From 94170066c30d1a3948da12a82422c282de3d97d0 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 24 Mar 2024 13:32:53 -0400 Subject: [PATCH 19/27] updated proto name Signed-off-by: Francisco Javier Arceo --- protos/feast/core/Transformation.proto | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/protos/feast/core/Transformation.proto b/protos/feast/core/Transformation.proto index cde2833fa4..36f1e691fe 100644 --- a/protos/feast/core/Transformation.proto +++ b/protos/feast/core/Transformation.proto @@ -21,13 +21,12 @@ message UserDefinedFunctionV2 { // A feature transformation executed as a user-defined function message FeatureTransformationV2 { - // Note this Transformation starts at 5 for backwards compatibility oneof transformation { UserDefinedFunctionV2 user_defined_function = 1; - OnDemandSubstraitTransformationV2 on_demand_substrait_transformation = 2; + SubstraitTransformationV2 substrait_transformation = 2; } } -message OnDemandSubstraitTransformationV2 { +message SubstraitTransformationV2 { bytes substrait_plan = 1; } From eff14977e56bd691178953b0868315cc510b2d2c Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 24 Mar 2024 13:44:30 -0400 Subject: [PATCH 20/27] renamed substrait proto Signed-off-by: Francisco Javier Arceo --- .../feast/transformation/substrait_transformation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/python/feast/transformation/substrait_transformation.py b/sdk/python/feast/transformation/substrait_transformation.py index 4b96541c8a..82fd407c89 100644 --- a/sdk/python/feast/transformation/substrait_transformation.py +++ b/sdk/python/feast/transformation/substrait_transformation.py @@ -3,7 +3,7 @@ import pyarrow.substrait as substrait # type: ignore # noqa from feast.protos.feast.core.Transformation_pb2 import ( - OnDemandSubstraitTransformationV2 as OnDemandSubstraitTransformationProto, + SubstraitTransformationV2 as SubstraitTransformationProto, ) @@ -37,13 +37,13 @@ def __eq__(self, other): return self.substrait_plan == other.substrait_plan - def to_proto(self) -> OnDemandSubstraitTransformationProto: - return OnDemandSubstraitTransformationProto(substrait_plan=self.substrait_plan) + def to_proto(self) -> SubstraitTransformationProto: + return SubstraitTransformationProto(substrait_plan=self.substrait_plan) @classmethod def from_proto( cls, - on_demand_substrait_transformation_proto: OnDemandSubstraitTransformationProto, + on_demand_substrait_transformation_proto: SubstraitTransformationProto, ): return SubstraitTransformation( substrait_plan=on_demand_substrait_transformation_proto.substrait_plan From ae19919bf02ffc8c9ff9218d0a176d8846070ec3 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 24 Mar 2024 13:46:49 -0400 Subject: [PATCH 21/27] renamed substrait proto Signed-off-by: Francisco Javier Arceo --- .../feast/transformation/substrait_transformation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/python/feast/transformation/substrait_transformation.py b/sdk/python/feast/transformation/substrait_transformation.py index 82fd407c89..b3dbe7a4b4 100644 --- a/sdk/python/feast/transformation/substrait_transformation.py +++ b/sdk/python/feast/transformation/substrait_transformation.py @@ -10,7 +10,7 @@ class SubstraitTransformation: def __init__(self, substrait_plan: bytes): """ - Creates an OnDemandSubstraitTransformation object. + Creates an SubstraitTransformation object. Args: substrait_plan: The user-provided substrait plan. @@ -29,7 +29,7 @@ def table_provider(names, schema: pyarrow.Schema): def __eq__(self, other): if not isinstance(other, SubstraitTransformation): raise TypeError( - "Comparisons should only involve OnDemandSubstraitTransformation class objects." + "Comparisons should only involve SubstraitTransformation class objects." ) if not super().__eq__(other): @@ -43,8 +43,8 @@ def to_proto(self) -> SubstraitTransformationProto: @classmethod def from_proto( cls, - on_demand_substrait_transformation_proto: SubstraitTransformationProto, + substrait_transformation_proto: SubstraitTransformationProto, ): return SubstraitTransformation( - substrait_plan=on_demand_substrait_transformation_proto.substrait_plan + substrait_plan=substrait_transformation_proto.substrait_plan ) From e34b604a2cb7304b5801d042683368dfcc106e92 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Sun, 24 Mar 2024 14:25:10 -0400 Subject: [PATCH 22/27] updated --- sdk/python/feast/on_demand_feature_view.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 9ca76a2a72..63ddf9607e 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -221,7 +221,7 @@ def to_proto(self) -> OnDemandFeatureViewProto: user_defined_function=self.transformation.to_proto() if type(self.transformation) == PandasTransformation else None, - on_demand_substrait_transformation=self.transformation.to_proto() + substrait_transformation=self.transformation.to_proto() if type(self.transformation) == SubstraitTransformation else None, # type: ignore ) @@ -283,10 +283,10 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): on_demand_feature_view_proto.spec.feature_transformation.WhichOneof( "transformation" ) - == "on_demand_substrait_transformation" + == "substrait_transformation" ): transformation = SubstraitTransformation.from_proto( - on_demand_feature_view_proto.spec.feature_transformation.on_demand_substrait_transformation + on_demand_feature_view_proto.spec.feature_transformation.substrait_transformation ) elif ( hasattr(on_demand_feature_view_proto.spec, "user_defined_function") From 9cd0ebe5710f9c621b68f937dbfe499c1cfa988e Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Mon, 25 Mar 2024 00:01:40 -0400 Subject: [PATCH 23/27] updated Signed-off-by: Francisco Javier Arceo --- protos/feast/core/OnDemandFeatureView.proto | 12 ++----- sdk/python/feast/diff/registry_diff.py | 12 +++++-- sdk/python/feast/on_demand_feature_view.py | 32 ++++++++----------- .../transformation/pandas_transformation.py | 2 +- .../tests/unit/test_on_demand_feature_view.py | 30 ++++++++++------- 5 files changed, 44 insertions(+), 44 deletions(-) diff --git a/protos/feast/core/OnDemandFeatureView.proto b/protos/feast/core/OnDemandFeatureView.proto index cd3ceba150..7a5fec1650 100644 --- a/protos/feast/core/OnDemandFeatureView.proto +++ b/protos/feast/core/OnDemandFeatureView.proto @@ -49,10 +49,8 @@ message OnDemandFeatureViewSpec { // Map of sources for this feature view. map sources = 4; - oneof transformation { - UserDefinedFunction user_defined_function = 5 [deprecated = true]; - OnDemandSubstraitTransformation on_demand_substrait_transformation = 9 [deprecated = true]; - } + UserDefinedFunction user_defined_function = 5 [deprecated = true]; + // Oneof with {user_defined_function, on_demand_substrait_transformation} FeatureTransformationV2 feature_transformation = 10; @@ -96,9 +94,3 @@ message UserDefinedFunction { // The string representation of the udf string body_text = 3; } - -message OnDemandSubstraitTransformation { - option deprecated = true; - - bytes substrait_plan = 1; -} diff --git a/sdk/python/feast/diff/registry_diff.py b/sdk/python/feast/diff/registry_diff.py index 120f5d697a..7eaa06b0c9 100644 --- a/sdk/python/feast/diff/registry_diff.py +++ b/sdk/python/feast/diff/registry_diff.py @@ -146,7 +146,7 @@ def diff_registry_objects( continue elif getattr(current_spec, _field.name) != getattr(new_spec, _field.name): # TODO: Delete "transformation" after we've safely deprecated it from the proto - if _field.name in ["transformation", "feature_transformation"]: + if _field.name == "feature_transformation": warnings.warn( "transformation will be deprecated in the future please use feature_transformation instead.", DeprecationWarning, @@ -154,13 +154,19 @@ def diff_registry_objects( current_spec = cast(OnDemandFeatureViewSpec, current_spec) new_spec = cast(OnDemandFeatureViewSpec, new_spec) # Check if the old proto is populated and use that if it is - deprecated_udf = current_spec.user_defined_function feature_transformation_udf = ( current_spec.feature_transformation.user_defined_function ) + if ( + current_spec.HasField("user_defined_function") + and not feature_transformation_udf + ): + deprecated_udf = current_spec.user_defined_function + else: + deprecated_udf = None current_udf = ( deprecated_udf - if deprecated_udf.body_text != "" + if deprecated_udf is not None else feature_transformation_udf ) new_udf = new_spec.feature_transformation.user_defined_function diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 63ddf9607e..59e6138240 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -68,7 +68,6 @@ class OnDemandFeatureView(BaseFeatureView): features: List[Field] source_feature_view_projections: Dict[str, FeatureViewProjection] source_request_sources: Dict[str, RequestSource] - transformation: Union[PandasTransformation] feature_transformation: Union[PandasTransformation] description: str tags: Dict[str, str] @@ -89,7 +88,6 @@ def __init__( # noqa: C901 ], udf: Optional[FunctionType] = None, udf_string: str = "", - transformation: Optional[Union[PandasTransformation]] = None, feature_transformation: Optional[Union[PandasTransformation]] = None, description: str = "", tags: Optional[Dict[str, str]] = None, @@ -108,7 +106,6 @@ def __init__( # noqa: C901 udf (deprecated): The user defined transformation function, which must take pandas dataframes as inputs. udf_string (deprecated): The source code version of the udf (for diffing and displaying in Web UI) - transformation: The user defined transformation. feature_transformation: The user defined transformation. description (optional): A human-readable description. tags (optional): A dictionary of key-value pairs to store arbitrary metadata. @@ -123,13 +120,13 @@ def __init__( # noqa: C901 owner=owner, ) - if not transformation: + if not feature_transformation: if udf: warnings.warn( "udf and udf_string parameters are deprecated. Please use transformation=OnDemandPandasTransformation(udf, udf_string) instead.", DeprecationWarning, ) - transformation = PandasTransformation(udf, udf_string) + feature_transformation = PandasTransformation(udf, udf_string) else: raise Exception( "OnDemandFeatureView needs to be initialized with either transformation or udf arguments" @@ -147,8 +144,7 @@ def __init__( # noqa: C901 odfv_source.name ] = odfv_source.projection - self.transformation = transformation - self.feature_transformation = self.transformation + self.feature_transformation = feature_transformation @property def proto_class(self) -> Type[OnDemandFeatureViewProto]: @@ -160,8 +156,7 @@ def __copy__(self): schema=self.features, sources=list(self.source_feature_view_projections.values()) + list(self.source_request_sources.values()), - transformation=self.transformation, - feature_transformation=self.transformation, + feature_transformation=self.feature_transformation, description=self.description, tags=self.tags, owner=self.owner, @@ -182,7 +177,6 @@ def __eq__(self, other): self.source_feature_view_projections != other.source_feature_view_projections or self.source_request_sources != other.source_request_sources - or self.transformation != other.transformation or self.feature_transformation != other.feature_transformation ): return False @@ -218,11 +212,11 @@ def to_proto(self) -> OnDemandFeatureViewProto: ) feature_transformation = FeatureTransformationProto( - user_defined_function=self.transformation.to_proto() - if type(self.transformation) == PandasTransformation + user_defined_function=self.feature_transformation.to_proto() + if type(self.feature_transformation) == PandasTransformation else None, - substrait_transformation=self.transformation.to_proto() - if type(self.transformation) == SubstraitTransformation + substrait_transformation=self.feature_transformation.to_proto() + if type(self.feature_transformation) == SubstraitTransformation else None, # type: ignore ) spec = OnDemandFeatureViewSpec( @@ -314,7 +308,7 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto): for feature in on_demand_feature_view_proto.spec.features ], sources=sources, - transformation=transformation, + feature_transformation=transformation, description=on_demand_feature_view_proto.spec.description, tags=dict(on_demand_feature_view_proto.spec.tags), owner=on_demand_feature_view_proto.spec.owner, @@ -374,7 +368,9 @@ def get_transformed_features_df( # Compute transformed values and apply to each result row - df_with_transformed_features = self.transformation.transform(df_with_features) + df_with_transformed_features = self.feature_transformation.transform( + df_with_features + ) # Work out whether the correct columns names are used. rename_columns: Dict[str, str] = {} @@ -424,7 +420,7 @@ def infer_features(self) -> None: dtype = feast_value_type_to_pandas_type(field.dtype.to_value_type()) sample_val = rand_df_value[dtype] if dtype in rand_df_value else None df[f"{field.name}"] = pd.Series(sample_val, dtype=dtype) - output_df: pd.DataFrame = self.transformation.transform(df) + output_df: pd.DataFrame = self.feature_transformation.transform(df) inferred_features = [] for f, dt in zip(output_df.columns, output_df.dtypes): inferred_features.append( @@ -552,7 +548,7 @@ def decorator(user_function): name=user_function.__name__, sources=sources, schema=schema, - transformation=transformation, + feature_transformation=transformation, description=description, tags=tags, owner=owner, diff --git a/sdk/python/feast/transformation/pandas_transformation.py b/sdk/python/feast/transformation/pandas_transformation.py index c6b3eed1b3..76f17e2106 100644 --- a/sdk/python/feast/transformation/pandas_transformation.py +++ b/sdk/python/feast/transformation/pandas_transformation.py @@ -27,7 +27,7 @@ def transform(self, df: pd.DataFrame) -> pd.DataFrame: def __eq__(self, other): if not isinstance(other, PandasTransformation): raise TypeError( - "Comparisons should only involve OnDemandPandasTransformation class objects." + "Comparisons should only involve PandasTransformation class objects." ) if ( diff --git a/sdk/python/tests/unit/test_on_demand_feature_view.py b/sdk/python/tests/unit/test_on_demand_feature_view.py index 8cea3be352..d561bd8e84 100644 --- a/sdk/python/tests/unit/test_on_demand_feature_view.py +++ b/sdk/python/tests/unit/test_on_demand_feature_view.py @@ -56,7 +56,9 @@ def test_hash(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=PandasTransformation(udf=udf1, udf_string="udf1 source code"), + feature_transformation=PandasTransformation( + udf=udf1, udf_string="udf1 source code" + ), ) on_demand_feature_view_2 = OnDemandFeatureView( name="my-on-demand-feature-view", @@ -65,7 +67,9 @@ def test_hash(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=PandasTransformation(udf=udf1, udf_string="udf1 source code"), + feature_transformation=PandasTransformation( + udf=udf1, udf_string="udf1 source code" + ), ) on_demand_feature_view_3 = OnDemandFeatureView( name="my-on-demand-feature-view", @@ -74,7 +78,9 @@ def test_hash(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=PandasTransformation(udf=udf2, udf_string="udf2 source code"), + feature_transformation=PandasTransformation( + udf=udf2, udf_string="udf2 source code" + ), ) on_demand_feature_view_4 = OnDemandFeatureView( name="my-on-demand-feature-view", @@ -83,7 +89,9 @@ def test_hash(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=PandasTransformation(udf=udf2, udf_string="udf2 source code"), + feature_transformation=PandasTransformation( + udf=udf2, udf_string="udf2 source code" + ), description="test", ) on_demand_feature_view_5 = OnDemandFeatureView( @@ -115,17 +123,13 @@ def test_hash(): } assert len(s4) == 3 - assert on_demand_feature_view_5.transformation == PandasTransformation( + assert on_demand_feature_view_5.feature_transformation == PandasTransformation( udf2, "udf2 source code" ) - assert ( - on_demand_feature_view_5.feature_transformation - == on_demand_feature_view_5.transformation - ) @pytest.mark.filterwarnings("ignore:udf and udf_string parameters are deprecated") -def test_from_proto_backwards_compatable_udf(): +def test_from_proto_backwards_compatible_udf(): file_source = FileSource(name="my-file-source", path="test.parquet") feature_view = FeatureView( name="my-feature-view", @@ -144,14 +148,16 @@ def test_from_proto_backwards_compatable_udf(): Field(name="output1", dtype=Float32), Field(name="output2", dtype=Float32), ], - transformation=PandasTransformation(udf=udf1, udf_string="udf1 source code"), + feature_transformation=PandasTransformation( + udf=udf1, udf_string="udf1 source code" + ), ) # We need a proto with the "udf1 source code" in the user_defined_function.body_text # and to populate it in feature_transformation proto = on_demand_feature_view.to_proto() assert ( - on_demand_feature_view.transformation.udf_string + on_demand_feature_view.feature_transformation.udf_string == proto.spec.feature_transformation.user_defined_function.body_text ) # Because of the current set of code this is just confirming it is empty From 7b9f1804cffab4e27e5d9f9b32633ce6e0d8da1f Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Mon, 25 Mar 2024 00:09:39 -0400 Subject: [PATCH 24/27] updated integration test --- .../tests/integration/feature_repos/universal/feature_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/feature_repos/universal/feature_views.py b/sdk/python/tests/integration/feature_repos/universal/feature_views.py index 6ab01901da..372cd8c152 100644 --- a/sdk/python/tests/integration/feature_repos/universal/feature_views.py +++ b/sdk/python/tests/integration/feature_repos/universal/feature_views.py @@ -71,7 +71,7 @@ def conv_rate_plus_100_feature_view( name=conv_rate_plus_100.__name__, schema=[] if infer_features else _features, sources=sources, - transformation=PandasTransformation( + feature_transformation=PandasTransformation( udf=conv_rate_plus_100, udf_string="raw udf source" ), ) From 19544f4fd9916d255da3cfc00e5ce5c3061ef139 Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Mon, 25 Mar 2024 00:10:45 -0400 Subject: [PATCH 25/27] missed one Signed-off-by: Francisco Javier Arceo --- .../tests/integration/feature_repos/universal/feature_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/feature_repos/universal/feature_views.py b/sdk/python/tests/integration/feature_repos/universal/feature_views.py index 372cd8c152..55d2ed8425 100644 --- a/sdk/python/tests/integration/feature_repos/universal/feature_views.py +++ b/sdk/python/tests/integration/feature_repos/universal/feature_views.py @@ -110,7 +110,7 @@ def similarity_feature_view( name=similarity.__name__, sources=sources, schema=[] if infer_features else _fields, - transformation=PandasTransformation( + feature_transformation=PandasTransformation( udf=similarity, udf_string="similarity raw udf" ), ) From 41524c9cd23b69cd31eb0e63b893aa635e00068f Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Mon, 25 Mar 2024 12:42:26 -0400 Subject: [PATCH 26/27] updated to include Substrait type --- sdk/python/feast/diff/registry_diff.py | 6 ----- .../feast/infra/registry/base_registry.py | 24 +++++++++++++++---- sdk/python/feast/on_demand_feature_view.py | 16 +++++++------ 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/sdk/python/feast/diff/registry_diff.py b/sdk/python/feast/diff/registry_diff.py index 7eaa06b0c9..41b2142226 100644 --- a/sdk/python/feast/diff/registry_diff.py +++ b/sdk/python/feast/diff/registry_diff.py @@ -1,4 +1,3 @@ -import warnings from dataclasses import dataclass from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, TypeVar, cast @@ -145,12 +144,7 @@ def diff_registry_objects( if _field.name in FIELDS_TO_IGNORE: continue elif getattr(current_spec, _field.name) != getattr(new_spec, _field.name): - # TODO: Delete "transformation" after we've safely deprecated it from the proto if _field.name == "feature_transformation": - warnings.warn( - "transformation will be deprecated in the future please use feature_transformation instead.", - DeprecationWarning, - ) current_spec = cast(OnDemandFeatureViewSpec, current_spec) new_spec = cast(OnDemandFeatureViewSpec, new_spec) # Check if the old proto is populated and use that if it is diff --git a/sdk/python/feast/infra/registry/base_registry.py b/sdk/python/feast/infra/registry/base_registry.py index d3d82a80b0..c90e49d00c 100644 --- a/sdk/python/feast/infra/registry/base_registry.py +++ b/sdk/python/feast/infra/registry/base_registry.py @@ -33,6 +33,8 @@ from feast.request_feature_view import RequestFeatureView from feast.saved_dataset import SavedDataset, ValidationReference from feast.stream_feature_view import StreamFeatureView +from feast.transformation.pandas_transformation import PandasTransformation +from feast.transformation.substrait_transformation import SubstraitTransformation class BaseRegistry(ABC): @@ -670,10 +672,24 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]: "We will be deprecating the usage of spec.userDefinedFunction in a future release please upgrade cautiously.", DeprecationWarning, ) - odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"][ - "body" - ] = on_demand_feature_view.feature_transformation.udf_string - registry_dict["onDemandFeatureViews"].append(odfv_dict) + if on_demand_feature_view.feature_transformation: + if isinstance(on_demand_feature_view.feature_transformation, PandasTransformation): + if "userDefinedFunction" not in odfv_dict["spec"]: + odfv_dict["spec"]["userDefinedFunction"] = {} + odfv_dict["spec"]["userDefinedFunction"][ + "body" + ] = on_demand_feature_view.feature_transformation.udf_string + odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"][ + "body" + ] = on_demand_feature_view.feature_transformation.udf_string + elif isinstance(on_demand_feature_view.feature_transformation, SubstraitTransformation): + odfv_dict["spec"]["featureTransformation"]["substraitPlan"]["body"] = ( + on_demand_feature_view.feature_transformation.substrait_plan + ) + else: + odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"]["body"] = None + odfv_dict["spec"]["featureTransformation"]["substraitPlan"]["body"] = None + registry_dict["onDemandFeatureViews"].append(odfv_dict) for request_feature_view in sorted( self.list_request_feature_views(project=project), key=lambda request_feature_view: request_feature_view.name, diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 59e6138240..ce416fff2a 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -57,7 +57,7 @@ class OnDemandFeatureView(BaseFeatureView): sources with type FeatureViewProjection. source_request_sources: A map from input source names to the actual input sources with type RequestSource. - transformation: The user defined transformation. + feature_transformation: The user defined transformation. description: A human-readable description. tags: A dictionary of key-value pairs to store arbitrary metadata. owner: The owner of the on demand feature view, typically the email of the primary @@ -68,7 +68,7 @@ class OnDemandFeatureView(BaseFeatureView): features: List[Field] source_feature_view_projections: Dict[str, FeatureViewProjection] source_request_sources: Dict[str, RequestSource] - feature_transformation: Union[PandasTransformation] + feature_transformation: Union[PandasTransformation, SubstraitTransformation] description: str tags: Dict[str, str] owner: str @@ -88,7 +88,9 @@ def __init__( # noqa: C901 ], udf: Optional[FunctionType] = None, udf_string: str = "", - feature_transformation: Optional[Union[PandasTransformation]] = None, + feature_transformation: Optional[ + Union[PandasTransformation, SubstraitTransformation] + ] = None, description: str = "", tags: Optional[Dict[str, str]] = None, owner: str = "", @@ -213,11 +215,11 @@ def to_proto(self) -> OnDemandFeatureViewProto: feature_transformation = FeatureTransformationProto( user_defined_function=self.feature_transformation.to_proto() - if type(self.feature_transformation) == PandasTransformation + if isinstance(self.feature_transformation, PandasTransformation) else None, substrait_transformation=self.feature_transformation.to_proto() - if type(self.feature_transformation) == SubstraitTransformation - else None, # type: ignore + if isinstance(self.feature_transformation, SubstraitTransformation) + else None, ) spec = OnDemandFeatureViewSpec( name=self.name, @@ -517,7 +519,7 @@ def decorator(user_function): input_fields: Field = [] for s in sources: - if type(s) == FeatureView: + if isinstance(s, FeatureView): fields = s.projection.features else: fields = s.features From 7de39ab74b7a894c49306e98d9ff26f93e3ccdbf Mon Sep 17 00:00:00 2001 From: Francisco Javier Arceo Date: Mon, 25 Mar 2024 12:49:44 -0400 Subject: [PATCH 27/27] linter Signed-off-by: Francisco Javier Arceo --- .../feast/infra/registry/base_registry.py | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/sdk/python/feast/infra/registry/base_registry.py b/sdk/python/feast/infra/registry/base_registry.py index c90e49d00c..583206941e 100644 --- a/sdk/python/feast/infra/registry/base_registry.py +++ b/sdk/python/feast/infra/registry/base_registry.py @@ -673,7 +673,9 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]: DeprecationWarning, ) if on_demand_feature_view.feature_transformation: - if isinstance(on_demand_feature_view.feature_transformation, PandasTransformation): + if isinstance( + on_demand_feature_view.feature_transformation, PandasTransformation + ): if "userDefinedFunction" not in odfv_dict["spec"]: odfv_dict["spec"]["userDefinedFunction"] = {} odfv_dict["spec"]["userDefinedFunction"][ @@ -682,13 +684,20 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]: odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"][ "body" ] = on_demand_feature_view.feature_transformation.udf_string - elif isinstance(on_demand_feature_view.feature_transformation, SubstraitTransformation): - odfv_dict["spec"]["featureTransformation"]["substraitPlan"]["body"] = ( - on_demand_feature_view.feature_transformation.substrait_plan - ) + elif isinstance( + on_demand_feature_view.feature_transformation, + SubstraitTransformation, + ): + odfv_dict["spec"]["featureTransformation"]["substraitPlan"][ + "body" + ] = on_demand_feature_view.feature_transformation.substrait_plan else: - odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"]["body"] = None - odfv_dict["spec"]["featureTransformation"]["substraitPlan"]["body"] = None + odfv_dict["spec"]["featureTransformation"]["userDefinedFunction"][ + "body" + ] = None + odfv_dict["spec"]["featureTransformation"]["substraitPlan"][ + "body" + ] = None registry_dict["onDemandFeatureViews"].append(odfv_dict) for request_feature_view in sorted( self.list_request_feature_views(project=project),