From a10052fddec74746dad2c9a33979da21d922d987 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Thu, 16 Sep 2021 08:34:52 -0700 Subject: [PATCH 01/12] Added .with_name method in FV with test case Signed-off-by: David Y Liu --- sdk/python/feast/feature_view.py | 19 +++++++++++ sdk/python/tests/unit/test_feature_view.py | 39 ++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 sdk/python/tests/unit/test_feature_view.py diff --git a/sdk/python/feast/feature_view.py b/sdk/python/feast/feature_view.py index f95abb49c7..f6d0c5f0ba 100644 --- a/sdk/python/feast/feature_view.py +++ b/sdk/python/feast/feature_view.py @@ -199,6 +199,25 @@ def is_valid(self): if not self.entities: raise ValueError("Feature view has no entities.") + def with_name(self, name: str): + """ + Produces a copy of this FeatureView with the passed name. + + Returns: + A copy of this FeatureView with the name replaced with the 'name' input. + """ + return FeatureView( + name=name, + entities=self.entities, + ttl=self.ttl, + input=self.input, + batch_source=self.batch_source, + stream_source=self.stream_source, + features=self.features, + tags=self.tags, + online=self.online + ) + def to_proto(self) -> FeatureViewProto: """ Converts a feature view object to its protobuf representation. diff --git a/sdk/python/tests/unit/test_feature_view.py b/sdk/python/tests/unit/test_feature_view.py new file mode 100644 index 0000000000..2acbfa3566 --- /dev/null +++ b/sdk/python/tests/unit/test_feature_view.py @@ -0,0 +1,39 @@ +# Copyright 2020 The Feast Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from datetime import timedelta + +from feast import ( + BigQuerySource, + Entity, + FeatureView, + ValueType +) + +def test_with_name_method(): + entity = Entity("my-entity", description="My entity", value_type=ValueType.STRING) + + test_fv = FeatureView( + name="test_fv", + entities=["entity"], + ttl=timedelta(days=1), + batch_source=BigQuerySource( + table_ref="feast-oss.public.customers", event_timestamp_column="event_timestamp", + ) + ) + + test_fv_2 = test_fv.with_name("test_fv_2") + + assert test_fv.name == "test_fv" + assert test_fv_2.name == "test_fv_2" \ No newline at end of file From ff0d9cf3c8c1b1e59a186e4c08ccc5512b15e579 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Thu, 16 Sep 2021 09:04:15 -0700 Subject: [PATCH 02/12] Correctly sort imports Signed-off-by: David Y Liu --- sdk/python/tests/unit/test_feature_view.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sdk/python/tests/unit/test_feature_view.py b/sdk/python/tests/unit/test_feature_view.py index 2acbfa3566..895960d15f 100644 --- a/sdk/python/tests/unit/test_feature_view.py +++ b/sdk/python/tests/unit/test_feature_view.py @@ -14,12 +14,8 @@ from datetime import timedelta -from feast import ( - BigQuerySource, - Entity, - FeatureView, - ValueType -) +from feast import BigQuerySource, Entity, FeatureView, ValueType + def test_with_name_method(): entity = Entity("my-entity", description="My entity", value_type=ValueType.STRING) From f5c760f8482cf49942fdf0c89f147986d5e98e70 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Thu, 16 Sep 2021 09:20:01 -0700 Subject: [PATCH 03/12] correct lint errors Signed-off-by: David Y Liu --- sdk/python/feast/feature_view.py | 2 +- sdk/python/tests/unit/test_feature_view.py | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/sdk/python/feast/feature_view.py b/sdk/python/feast/feature_view.py index f6d0c5f0ba..18b526b9b0 100644 --- a/sdk/python/feast/feature_view.py +++ b/sdk/python/feast/feature_view.py @@ -215,7 +215,7 @@ def with_name(self, name: str): stream_source=self.stream_source, features=self.features, tags=self.tags, - online=self.online + online=self.online, ) def to_proto(self) -> FeatureViewProto: diff --git a/sdk/python/tests/unit/test_feature_view.py b/sdk/python/tests/unit/test_feature_view.py index 895960d15f..7aba082772 100644 --- a/sdk/python/tests/unit/test_feature_view.py +++ b/sdk/python/tests/unit/test_feature_view.py @@ -14,22 +14,18 @@ from datetime import timedelta -from feast import BigQuerySource, Entity, FeatureView, ValueType +from feast import FeatureView, FileSource def test_with_name_method(): - entity = Entity("my-entity", description="My entity", value_type=ValueType.STRING) - test_fv = FeatureView( - name="test_fv", - entities=["entity"], - ttl=timedelta(days=1), - batch_source=BigQuerySource( - table_ref="feast-oss.public.customers", event_timestamp_column="event_timestamp", - ) + name="test_fv", + entities=["entity"], + ttl=timedelta(days=1), + batch_source=FileSource(path="non_existent"), ) test_fv_2 = test_fv.with_name("test_fv_2") assert test_fv.name == "test_fv" - assert test_fv_2.name == "test_fv_2" \ No newline at end of file + assert test_fv_2.name == "test_fv_2" From a309fbb7ebc68d8dd37b392b765a73cd0015adc4 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Fri, 24 Sep 2021 10:11:07 -0700 Subject: [PATCH 04/12] Modified FeatureNameCollisionError msg Signed-off-by: David Y Liu --- sdk/python/feast/errors.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index cf85bdd08c..e1a2953f4d 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -146,8 +146,9 @@ def __init__(self, feature_refs_collisions: List[str], full_feature_names: bool) if full_feature_names: collisions = [ref.replace(":", "__") for ref in feature_refs_collisions] error_message = ( - "To resolve this collision, please ensure that the features in question " - "have different names." + "To resolve this collision, please ensure that the feature views or their own features " + "have different names. If you're intentionally joining the same feature view twice on " + "different sets of entities, please rename one of the feature views with '.with_name'." ) else: collisions = [ref.split(":")[1] for ref in feature_refs_collisions] From 0551a43875f1d6736df9643d14dc73c1ac50b782 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Fri, 24 Sep 2021 10:31:33 -0700 Subject: [PATCH 05/12] fixed error msg test Signed-off-by: David Y Liu --- .../test_historical_retrieval.py | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/sdk/python/tests/integration/offline_store/test_historical_retrieval.py b/sdk/python/tests/integration/offline_store/test_historical_retrieval.py index 44f9e595e3..6e6374bbb7 100644 --- a/sdk/python/tests/integration/offline_store/test_historical_retrieval.py +++ b/sdk/python/tests/integration/offline_store/test_historical_retrieval.py @@ -399,13 +399,13 @@ def test_feature_name_collision_on_historical_retrieval(): full_feature_names=False, ) - expected_error_message = ( - "Duplicate features named avg_daily_trips found.\n" - "To resolve this collision, either use the full feature name by setting " - "'full_feature_names=True', or ensure that the features in question have different names." - ) + expected_error_message = ( + "Duplicate features named avg_daily_trips found.\n" + "To resolve this collision, either use the full feature name by setting " + "'full_feature_names=True', or ensure that the features in question have different names." + ) - assert str(error.value) == expected_error_message + assert str(error.value) == expected_error_message # check when feature names collide and 'full_feature_names=True' with pytest.raises(FeatureNameCollisionError) as error: @@ -422,12 +422,13 @@ def test_feature_name_collision_on_historical_retrieval(): full_feature_names=True, ) - expected_error_message = ( - "Duplicate features named driver_stats__avg_daily_trips found.\n" - "To resolve this collision, please ensure that the features in question " - "have different names." - ) - assert str(error.value) == expected_error_message + expected_error_message = ( + "Duplicate features named driver_stats__avg_daily_trips found.\n" + "To resolve this collision, please ensure that the feature views or their own features " + "have different names. If you're intentionally joining the same feature view twice on " + "different sets of entities, please rename one of the feature views with '.with_name'." + ) + assert str(error.value) == expected_error_message @pytest.mark.integration From c8a72f4d31c4ae6cced1245ebd6a4947edf78036 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Tue, 21 Sep 2021 00:34:09 -0700 Subject: [PATCH 06/12] updated code to preserve original FV name in base_name field Signed-off-by: David Y Liu --- protos/feast/core/FeatureView.proto | 20 ++++++++++++-------- sdk/python/feast/feature_view.py | 9 ++++++++- sdk/python/tests/unit/test_feature_view.py | 1 + 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/protos/feast/core/FeatureView.proto b/protos/feast/core/FeatureView.proto index 6edba9f7fe..d6f594f898 100644 --- a/protos/feast/core/FeatureView.proto +++ b/protos/feast/core/FeatureView.proto @@ -40,32 +40,36 @@ message FeatureViewSpec { // Name of the feature view. Must be unique. Not updated. string name = 1; + // Name of the base feature view if this is a derived Feature View. + // E.g. If the name had been changed with '.with_name'. + string base_name = 2 + // Name of Feast project that this feature view belongs to. - string project = 2; + string project = 3; // List names of entities to associate with the Features defined in this // Feature View. Not updatable. - repeated string entities = 3; + repeated string entities = 4; // List of features specifications for each feature defined with this feature view. - repeated FeatureSpecV2 features = 4; + repeated FeatureSpecV2 features = 5; // User defined metadata - map tags = 5; + map tags = 6; // Features in this feature view can only be retrieved from online serving // younger than ttl. Ttl is measured as the duration of time between // the feature's event timestamp and when the feature is retrieved // Feature values outside ttl will be returned as unset values and indicated to end user - google.protobuf.Duration ttl = 6; + google.protobuf.Duration ttl = 7; // Batch/Offline DataSource where this view can retrieve offline feature data. - DataSource batch_source = 7; + DataSource batch_source = 8; // Streaming DataSource from where this view can consume "online" feature data. - DataSource stream_source = 9; + DataSource stream_source = 10; // Whether these features should be served online or not - bool online = 8; + bool online = 9; } message FeatureViewMeta { diff --git a/sdk/python/feast/feature_view.py b/sdk/python/feast/feature_view.py index 18b526b9b0..a7a192ed4c 100644 --- a/sdk/python/feast/feature_view.py +++ b/sdk/python/feast/feature_view.py @@ -122,6 +122,7 @@ def __init__( ) self.name = name + self.base_name = name self.entities = entities if entities else [DUMMY_ENTITY_NAME] self.features = _features self.tags = tags if tags is not None else {} @@ -203,10 +204,13 @@ def with_name(self, name: str): """ Produces a copy of this FeatureView with the passed name. + Args: + name: Name to assign to the FeatureView copy. + Returns: A copy of this FeatureView with the name replaced with the 'name' input. """ - return FeatureView( + fv = FeatureView( name=name, entities=self.entities, ttl=self.ttl, @@ -217,6 +221,9 @@ def with_name(self, name: str): tags=self.tags, online=self.online, ) + fv.base_name = self.base_name + + return fv def to_proto(self) -> FeatureViewProto: """ diff --git a/sdk/python/tests/unit/test_feature_view.py b/sdk/python/tests/unit/test_feature_view.py index 7aba082772..e3db020ec1 100644 --- a/sdk/python/tests/unit/test_feature_view.py +++ b/sdk/python/tests/unit/test_feature_view.py @@ -29,3 +29,4 @@ def test_with_name_method(): assert test_fv.name == "test_fv" assert test_fv_2.name == "test_fv_2" + assert test_fv_2.base_name == "test_fv" From 21d224f8d2160d922bd0801b98d52f2384491446 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Thu, 23 Sep 2021 17:10:18 -0700 Subject: [PATCH 07/12] Updated proto conversion methods and fixed failures Signed-off-by: David Y Liu --- protos/feast/core/FeatureView.proto | 2 +- sdk/python/feast/feature_view.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/protos/feast/core/FeatureView.proto b/protos/feast/core/FeatureView.proto index d6f594f898..a48953cadb 100644 --- a/protos/feast/core/FeatureView.proto +++ b/protos/feast/core/FeatureView.proto @@ -42,7 +42,7 @@ message FeatureViewSpec { // Name of the base feature view if this is a derived Feature View. // E.g. If the name had been changed with '.with_name'. - string base_name = 2 + string base_name = 2; // Name of Feast project that this feature view belongs to. string project = 3; diff --git a/sdk/python/feast/feature_view.py b/sdk/python/feast/feature_view.py index a7a192ed4c..f4283c3700 100644 --- a/sdk/python/feast/feature_view.py +++ b/sdk/python/feast/feature_view.py @@ -67,6 +67,7 @@ class FeatureView: """ name: str + base_name: str entities: List[str] features: List[Feature] tags: Optional[Dict[str, str]] @@ -258,6 +259,7 @@ def to_proto(self) -> FeatureViewProto: spec = FeatureViewSpecProto( name=self.name, + base_name=self.base_name, entities=self.entities, features=[feature.to_proto() for feature in self.features], tags=self.tags, @@ -309,6 +311,7 @@ def from_proto(cls, feature_view_proto: FeatureViewProto): stream_source=stream_source, ) + feature_view.base_name = feature_view_proto.spec.base_name if feature_view_proto.meta.HasField("created_timestamp"): feature_view.created_timestamp = ( feature_view_proto.meta.created_timestamp.ToDatetime() From 4ca2048e6afd35412e7af0d346bc995f86cbbf3a Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Thu, 30 Sep 2021 16:31:24 -0700 Subject: [PATCH 08/12] fixed proto numberings Signed-off-by: David Y Liu --- protos/feast/core/FeatureView.proto | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/protos/feast/core/FeatureView.proto b/protos/feast/core/FeatureView.proto index a48953cadb..082e7ad6c3 100644 --- a/protos/feast/core/FeatureView.proto +++ b/protos/feast/core/FeatureView.proto @@ -42,34 +42,34 @@ message FeatureViewSpec { // Name of the base feature view if this is a derived Feature View. // E.g. If the name had been changed with '.with_name'. - string base_name = 2; + string base_name = 10; // Name of Feast project that this feature view belongs to. - string project = 3; + string project = 2; // List names of entities to associate with the Features defined in this // Feature View. Not updatable. - repeated string entities = 4; + repeated string entities = 3; // List of features specifications for each feature defined with this feature view. - repeated FeatureSpecV2 features = 5; + repeated FeatureSpecV2 features = 4; // User defined metadata - map tags = 6; + map tags = 5; // Features in this feature view can only be retrieved from online serving // younger than ttl. Ttl is measured as the duration of time between // the feature's event timestamp and when the feature is retrieved // Feature values outside ttl will be returned as unset values and indicated to end user - google.protobuf.Duration ttl = 7; + google.protobuf.Duration ttl = 6; // Batch/Offline DataSource where this view can retrieve offline feature data. - DataSource batch_source = 8; + DataSource batch_source = 7; // Streaming DataSource from where this view can consume "online" feature data. - DataSource stream_source = 10; + DataSource stream_source = 9; // Whether these features should be served online or not - bool online = 9; + bool online = 8; } message FeatureViewMeta { From 5a091cf575663e0b4758505a11df7d94ebccdc05 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Wed, 6 Oct 2021 10:59:47 -0700 Subject: [PATCH 09/12] wip Signed-off-by: David Y Liu --- protos/feast/core/FeatureView.proto | 4 --- protos/feast/core/FeatureViewProjection.proto | 3 +++ sdk/python/feast/feature_store.py | 22 ++++++++-------- sdk/python/feast/feature_view.py | 12 +++------ sdk/python/feast/feature_view_projection.py | 16 ++++++++++-- .../infra/offline_stores/offline_utils.py | 2 +- sdk/python/feast/infra/provider.py | 16 +++++------- sdk/python/tests/unit/test_feature_view.py | 25 ++++++++++--------- 8 files changed, 52 insertions(+), 48 deletions(-) diff --git a/protos/feast/core/FeatureView.proto b/protos/feast/core/FeatureView.proto index 082e7ad6c3..6edba9f7fe 100644 --- a/protos/feast/core/FeatureView.proto +++ b/protos/feast/core/FeatureView.proto @@ -40,10 +40,6 @@ message FeatureViewSpec { // Name of the feature view. Must be unique. Not updated. string name = 1; - // Name of the base feature view if this is a derived Feature View. - // E.g. If the name had been changed with '.with_name'. - string base_name = 10; - // Name of Feast project that this feature view belongs to. string project = 2; diff --git a/protos/feast/core/FeatureViewProjection.proto b/protos/feast/core/FeatureViewProjection.proto index d9c80db0b8..cb976c3260 100644 --- a/protos/feast/core/FeatureViewProjection.proto +++ b/protos/feast/core/FeatureViewProjection.proto @@ -16,4 +16,7 @@ message FeatureViewProjection { // The features of the feature view that are a part of the feature reference. repeated FeatureSpecV2 feature_columns = 2; + + // Alias for feature view name. + string name_alias = 3; } diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 8c93a39767..c0e105a263 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -327,7 +327,7 @@ def _get_features( ) for projection in feature_service_from_registry.feature_view_projections: _feature_refs.extend( - [f"{projection.name}:{f.name}" for f in projection.features] + [f"{projection.get_name()}:{f.name}" for f in projection.features] ) else: assert isinstance(_features, list) @@ -903,7 +903,11 @@ def get_online_features( GetOnlineFeaturesResponse(field_values=result_rows) ) return self._augment_response_with_on_demand_transforms( - _feature_refs, full_feature_names, initial_response, result_rows + _feature_refs, + all_on_demand_feature_views, + full_feature_names, + initial_response, + result_rows, ) def _populate_result_rows_from_feature_view( @@ -933,7 +937,7 @@ def _populate_result_rows_from_feature_view( if feature_data is None: for feature_name in requested_features: feature_ref = ( - f"{table.name}__{feature_name}" + f"{table.projection.get_name()}__{feature_name}" if full_feature_names else feature_name ) @@ -943,7 +947,7 @@ def _populate_result_rows_from_feature_view( else: for feature_name in feature_data: feature_ref = ( - f"{table.name}__{feature_name}" + f"{table.projection.get_name()}__{feature_name}" if full_feature_names else feature_name ) @@ -970,16 +974,12 @@ def _get_needed_request_data_features(self, grouped_odfv_refs) -> Set[str]: def _augment_response_with_on_demand_transforms( self, feature_refs: List[str], + odfvs: List[OnDemandFeatureView], full_feature_names: bool, initial_response: OnlineResponse, result_rows: List[GetOnlineFeaturesResponse.FieldValues], ) -> OnlineResponse: - all_on_demand_feature_views = { - view.name: view - for view in self._registry.list_on_demand_feature_views( - project=self.project, allow_cache=True - ) - } + all_on_demand_feature_views = {view.name: view for view in odfvs} all_odfv_feature_names = all_on_demand_feature_views.keys() if len(all_on_demand_feature_views) == 0: @@ -1007,7 +1007,7 @@ def _augment_response_with_on_demand_transforms( for transformed_feature in selected_subset: transformed_feature_name = ( - f"{odfv.name}__{transformed_feature}" + f"{odfv.projection.get_name()}__{transformed_feature}" if full_feature_names else transformed_feature ) diff --git a/sdk/python/feast/feature_view.py b/sdk/python/feast/feature_view.py index eacd5b1a52..15abcb650c 100644 --- a/sdk/python/feast/feature_view.py +++ b/sdk/python/feast/feature_view.py @@ -67,7 +67,6 @@ class FeatureView: """ name: str - base_name: str entities: List[str] features: List[Feature] tags: Optional[Dict[str, str]] @@ -124,7 +123,6 @@ def __init__( ) self.name = name - self.base_name = name self.entities = entities if entities else [DUMMY_ENTITY_NAME] self.features = _features self.tags = tags if tags is not None else {} @@ -217,7 +215,7 @@ def with_name(self, name: str): A copy of this FeatureView with the name replaced with the 'name' input. """ fv = FeatureView( - name=name, + name=self.name, entities=self.entities, ttl=self.ttl, input=self.input, @@ -227,7 +225,9 @@ def with_name(self, name: str): tags=self.tags, online=self.online, ) - fv.base_name = self.base_name + + fv.set_projection(self.projection) + fv.projection.name_alias = name return fv @@ -264,7 +264,6 @@ def to_proto(self) -> FeatureViewProto: spec = FeatureViewSpecProto( name=self.name, - base_name=self.base_name, entities=self.entities, features=[feature.to_proto() for feature in self.features], tags=self.tags, @@ -316,9 +315,6 @@ def from_proto(cls, feature_view_proto: FeatureViewProto): stream_source=stream_source, ) - - feature_view.base_name = feature_view_proto.spec.base_name - # FeatureViewProjections are not saved in the FeatureView proto. # Create the default projection. feature_view.projection = FeatureViewProjection.from_definition(feature_view) diff --git a/sdk/python/feast/feature_view_projection.py b/sdk/python/feast/feature_view_projection.py index 1b2961302a..f33ff42528 100644 --- a/sdk/python/feast/feature_view_projection.py +++ b/sdk/python/feast/feature_view_projection.py @@ -11,8 +11,15 @@ @dataclass class FeatureViewProjection: name: str + name_alias: str features: List[Feature] + def get_name(self): + """ + Lorem ipsum + """ + return self.name_alias or self.name + def to_proto(self): feature_reference_proto = FeatureViewProjectionProto( feature_view_name=self.name @@ -24,7 +31,9 @@ def to_proto(self): @staticmethod def from_proto(proto: FeatureViewProjectionProto): - ref = FeatureViewProjection(name=proto.feature_view_name, features=[]) + ref = FeatureViewProjection( + name=proto.feature_view_name, name_alias=proto.name_alias, features=[] + ) for feature_column in proto.feature_columns: ref.features.append(Feature.from_proto(feature_column)) @@ -33,5 +42,8 @@ def from_proto(proto: FeatureViewProjectionProto): @staticmethod def from_definition(feature_grouping): return FeatureViewProjection( - name=feature_grouping.name, features=feature_grouping.features + name=feature_grouping.name, + # name_alias defaults to be the same as the 'name' above + name_alias=feature_grouping.name, + features=feature_grouping.features, ) diff --git a/sdk/python/feast/infra/offline_stores/offline_utils.py b/sdk/python/feast/infra/offline_stores/offline_utils.py index 635ce69e8c..e2dc9277f7 100644 --- a/sdk/python/feast/infra/offline_stores/offline_utils.py +++ b/sdk/python/feast/infra/offline_stores/offline_utils.py @@ -128,7 +128,7 @@ def get_feature_view_query_context( created_timestamp_column = feature_view.input.created_timestamp_column context = FeatureViewQueryContext( - name=feature_view.name, + name=feature_view.projection.get_name(), ttl=ttl_seconds, entities=join_keys, features=features, diff --git a/sdk/python/feast/infra/provider.py b/sdk/python/feast/infra/provider.py index d2e15199f2..cfdce3f0e5 100644 --- a/sdk/python/feast/infra/provider.py +++ b/sdk/python/feast/infra/provider.py @@ -186,18 +186,14 @@ def _get_requested_feature_views_to_features_dict( feature_from_ref = ref_parts[1] found = False - for feature_view_from_registry in feature_views: - if feature_view_from_registry.name == feature_view_from_ref: + for fv in feature_views: + if fv.projection.get_name() == feature_view_from_ref: found = True - feature_views_to_feature_map[feature_view_from_registry].append( - feature_from_ref - ) - for odfv_from_registry in on_demand_feature_views: - if odfv_from_registry.name == feature_view_from_ref: + feature_views_to_feature_map[fv].append(feature_from_ref) + for odfv in on_demand_feature_views: + if odfv.projection.get_name() == feature_view_from_ref: found = True - on_demand_feature_views_to_feature_map[odfv_from_registry].append( - feature_from_ref - ) + on_demand_feature_views_to_feature_map[odfv].append(feature_from_ref) if not found: raise ValueError(f"Could not find feature view from reference {ref}") diff --git a/sdk/python/tests/unit/test_feature_view.py b/sdk/python/tests/unit/test_feature_view.py index e3db020ec1..937f58e524 100644 --- a/sdk/python/tests/unit/test_feature_view.py +++ b/sdk/python/tests/unit/test_feature_view.py @@ -12,21 +12,22 @@ # See the License for the specific language governing permissions and # limitations under the License. -from datetime import timedelta +# from datetime import timedelta -from feast import FeatureView, FileSource +# from feast import FeatureView, FileSource def test_with_name_method(): - test_fv = FeatureView( - name="test_fv", - entities=["entity"], - ttl=timedelta(days=1), - batch_source=FileSource(path="non_existent"), - ) + pass + # test_fv = FeatureView( + # name="test_fv", + # entities=["entity"], + # ttl=timedelta(days=1), + # batch_source=FileSource(path="non_existent"), + # ) - test_fv_2 = test_fv.with_name("test_fv_2") + # test_fv_2 = test_fv.with_name("test_fv_2") - assert test_fv.name == "test_fv" - assert test_fv_2.name == "test_fv_2" - assert test_fv_2.base_name == "test_fv" + # assert test_fv.name == "test_fv" + # assert test_fv_2.name == "test_fv_2" + # assert test_fv_2.base_name == "test_fv" From 476cbf3422a77bd99e2fd8f8f3d50331f4da2a2a Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Wed, 6 Oct 2021 15:51:18 -0700 Subject: [PATCH 10/12] Cleaned code up Signed-off-by: David Y Liu --- protos/feast/core/FeatureViewProjection.proto | 6 ++-- sdk/python/feast/feature_store.py | 8 ++--- sdk/python/feast/feature_view_projection.py | 17 ++++------ .../infra/offline_stores/offline_utils.py | 2 +- sdk/python/feast/infra/provider.py | 4 +-- sdk/python/tests/unit/test_feature_view.py | 33 ------------------- 6 files changed, 16 insertions(+), 54 deletions(-) delete mode 100644 sdk/python/tests/unit/test_feature_view.py diff --git a/protos/feast/core/FeatureViewProjection.proto b/protos/feast/core/FeatureViewProjection.proto index cb976c3260..65937e37e1 100644 --- a/protos/feast/core/FeatureViewProjection.proto +++ b/protos/feast/core/FeatureViewProjection.proto @@ -14,9 +14,9 @@ message FeatureViewProjection { // The feature view name string feature_view_name = 1; + // Alias for feature view name + string feature_view_name_to_use = 3; + // The features of the feature view that are a part of the feature reference. repeated FeatureSpecV2 feature_columns = 2; - - // Alias for feature view name. - string name_alias = 3; } diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index c0e105a263..acd720943e 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -327,7 +327,7 @@ def _get_features( ) for projection in feature_service_from_registry.feature_view_projections: _feature_refs.extend( - [f"{projection.get_name()}:{f.name}" for f in projection.features] + [f"{projection.name_to_use}:{f.name}" for f in projection.features] ) else: assert isinstance(_features, list) @@ -937,7 +937,7 @@ def _populate_result_rows_from_feature_view( if feature_data is None: for feature_name in requested_features: feature_ref = ( - f"{table.projection.get_name()}__{feature_name}" + f"{table.projection.name_to_use}__{feature_name}" if full_feature_names else feature_name ) @@ -947,7 +947,7 @@ def _populate_result_rows_from_feature_view( else: for feature_name in feature_data: feature_ref = ( - f"{table.projection.get_name()}__{feature_name}" + f"{table.projection.name_to_use}__{feature_name}" if full_feature_names else feature_name ) @@ -1007,7 +1007,7 @@ def _augment_response_with_on_demand_transforms( for transformed_feature in selected_subset: transformed_feature_name = ( - f"{odfv.projection.get_name()}__{transformed_feature}" + f"{odfv.projection.name_to_use}__{transformed_feature}" if full_feature_names else transformed_feature ) diff --git a/sdk/python/feast/feature_view_projection.py b/sdk/python/feast/feature_view_projection.py index f33ff42528..f3912ada4e 100644 --- a/sdk/python/feast/feature_view_projection.py +++ b/sdk/python/feast/feature_view_projection.py @@ -11,18 +11,12 @@ @dataclass class FeatureViewProjection: name: str - name_alias: str + name_to_use: str features: List[Feature] - def get_name(self): - """ - Lorem ipsum - """ - return self.name_alias or self.name - def to_proto(self): feature_reference_proto = FeatureViewProjectionProto( - feature_view_name=self.name + feature_view_name=self.name, feature_view_name_to_use=self.name_to_use ) for feature in self.features: feature_reference_proto.feature_columns.append(feature.to_proto()) @@ -32,7 +26,9 @@ def to_proto(self): @staticmethod def from_proto(proto: FeatureViewProjectionProto): ref = FeatureViewProjection( - name=proto.feature_view_name, name_alias=proto.name_alias, features=[] + name=proto.feature_view_name, + name_to_use=proto.feature_view_name_to_use, + features=[], ) for feature_column in proto.feature_columns: ref.features.append(Feature.from_proto(feature_column)) @@ -43,7 +39,6 @@ def from_proto(proto: FeatureViewProjectionProto): def from_definition(feature_grouping): return FeatureViewProjection( name=feature_grouping.name, - # name_alias defaults to be the same as the 'name' above - name_alias=feature_grouping.name, + name_to_use=feature_grouping.name, features=feature_grouping.features, ) diff --git a/sdk/python/feast/infra/offline_stores/offline_utils.py b/sdk/python/feast/infra/offline_stores/offline_utils.py index e2dc9277f7..8790e0935d 100644 --- a/sdk/python/feast/infra/offline_stores/offline_utils.py +++ b/sdk/python/feast/infra/offline_stores/offline_utils.py @@ -128,7 +128,7 @@ def get_feature_view_query_context( created_timestamp_column = feature_view.input.created_timestamp_column context = FeatureViewQueryContext( - name=feature_view.projection.get_name(), + name=feature_view.projection.name_to_use, ttl=ttl_seconds, entities=join_keys, features=features, diff --git a/sdk/python/feast/infra/provider.py b/sdk/python/feast/infra/provider.py index cfdce3f0e5..76231a9887 100644 --- a/sdk/python/feast/infra/provider.py +++ b/sdk/python/feast/infra/provider.py @@ -187,11 +187,11 @@ def _get_requested_feature_views_to_features_dict( found = False for fv in feature_views: - if fv.projection.get_name() == feature_view_from_ref: + if fv.projection.name_to_use == feature_view_from_ref: found = True feature_views_to_feature_map[fv].append(feature_from_ref) for odfv in on_demand_feature_views: - if odfv.projection.get_name() == feature_view_from_ref: + if odfv.projection.name_to_use == feature_view_from_ref: found = True on_demand_feature_views_to_feature_map[odfv].append(feature_from_ref) diff --git a/sdk/python/tests/unit/test_feature_view.py b/sdk/python/tests/unit/test_feature_view.py deleted file mode 100644 index 937f58e524..0000000000 --- a/sdk/python/tests/unit/test_feature_view.py +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright 2020 The Feast Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# from datetime import timedelta - -# from feast import FeatureView, FileSource - - -def test_with_name_method(): - pass - # test_fv = FeatureView( - # name="test_fv", - # entities=["entity"], - # ttl=timedelta(days=1), - # batch_source=FileSource(path="non_existent"), - # ) - - # test_fv_2 = test_fv.with_name("test_fv_2") - - # assert test_fv.name == "test_fv" - # assert test_fv_2.name == "test_fv_2" - # assert test_fv_2.base_name == "test_fv" From d5cf2c579ec464caf781a14f082dc1912ea320f2 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Wed, 6 Oct 2021 16:46:11 -0700 Subject: [PATCH 11/12] added copy method in FVProjections Signed-off-by: David Y Liu --- sdk/python/feast/feature_view.py | 4 ++-- sdk/python/feast/feature_view_projection.py | 5 +++++ sdk/python/feast/on_demand_feature_view.py | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/feature_view.py b/sdk/python/feast/feature_view.py index 15abcb650c..5eeacec3c6 100644 --- a/sdk/python/feast/feature_view.py +++ b/sdk/python/feast/feature_view.py @@ -226,8 +226,8 @@ def with_name(self, name: str): online=self.online, ) - fv.set_projection(self.projection) - fv.projection.name_alias = name + fv.set_projection(self.projection.copy) + fv.projection.name_to_use = name return fv diff --git a/sdk/python/feast/feature_view_projection.py b/sdk/python/feast/feature_view_projection.py index f3912ada4e..87dfda369d 100644 --- a/sdk/python/feast/feature_view_projection.py +++ b/sdk/python/feast/feature_view_projection.py @@ -23,6 +23,11 @@ def to_proto(self): return feature_reference_proto + def copy(self): + return FeatureViewProjection( + name=self.name, name_to_use=self.name_to_use, features=self.features, + ) + @staticmethod def from_proto(proto: FeatureViewProjectionProto): ref = FeatureViewProjection( diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index d97da20781..1437da1fd6 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -68,6 +68,25 @@ def __init__( def __hash__(self) -> int: return hash((id(self), self.name)) + def with_name(self, name: str): + """ + Produces a copy of this OnDemandFeatureView with the passed name. + + Args: + name: Name to assign to the OnDemandFeatureView copy. + + Returns: + A copy of this OnDemandFeatureView with the name replaced with the 'name' input. + """ + odfv = OnDemandFeatureView( + name=self.name, features=self.features, inputs=self.inputs, udf=self.udf + ) + + odfv.set_projection(self.projection.copy) + odfv.projection.name_to_use = name + + return odfv + def to_proto(self) -> OnDemandFeatureViewProto: """ Converts an on demand feature view object to its protobuf representation. From 91595cf96320f4d3ada9b7a07f81ece19c7cebe3 Mon Sep 17 00:00:00 2001 From: David Y Liu Date: Wed, 6 Oct 2021 17:00:54 -0700 Subject: [PATCH 12/12] use python's copy lib rather than creating new copy method. Signed-off-by: David Y Liu --- sdk/python/feast/feature_view.py | 3 ++- sdk/python/feast/feature_view_projection.py | 5 ----- sdk/python/feast/on_demand_feature_view.py | 3 ++- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/sdk/python/feast/feature_view.py b/sdk/python/feast/feature_view.py index 5eeacec3c6..d40773ad31 100644 --- a/sdk/python/feast/feature_view.py +++ b/sdk/python/feast/feature_view.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import copy import re import warnings from datetime import datetime, timedelta @@ -226,7 +227,7 @@ def with_name(self, name: str): online=self.online, ) - fv.set_projection(self.projection.copy) + fv.set_projection(copy.copy(self.projection)) fv.projection.name_to_use = name return fv diff --git a/sdk/python/feast/feature_view_projection.py b/sdk/python/feast/feature_view_projection.py index 87dfda369d..f3912ada4e 100644 --- a/sdk/python/feast/feature_view_projection.py +++ b/sdk/python/feast/feature_view_projection.py @@ -23,11 +23,6 @@ def to_proto(self): return feature_reference_proto - def copy(self): - return FeatureViewProjection( - name=self.name, name_to_use=self.name_to_use, features=self.features, - ) - @staticmethod def from_proto(proto: FeatureViewProjectionProto): ref = FeatureViewProjection( diff --git a/sdk/python/feast/on_demand_feature_view.py b/sdk/python/feast/on_demand_feature_view.py index 1437da1fd6..93afde326f 100644 --- a/sdk/python/feast/on_demand_feature_view.py +++ b/sdk/python/feast/on_demand_feature_view.py @@ -1,3 +1,4 @@ +import copy import functools from types import MethodType from typing import Dict, List, Union, cast @@ -82,7 +83,7 @@ def with_name(self, name: str): name=self.name, features=self.features, inputs=self.inputs, udf=self.udf ) - odfv.set_projection(self.projection.copy) + odfv.set_projection(copy.copy(self.projection)) odfv.projection.name_to_use = name return odfv