Skip to content

Commit 88cc47d

Browse files
achalsachals
authored
fix: Fix feature view __getitem__ for feature services (#2769)
* fix: Fix feature view __getitem__ for feature services Signed-off-by: Achal Shah <achals@gmail.com> * more fixes Signed-off-by: Achal Shah <achals@gmail.com>
1 parent 8a39743 commit 88cc47d

File tree

6 files changed

+63
-24
lines changed

6 files changed

+63
-24
lines changed

‎sdk/python/feast/base_feature_view.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,18 +110,20 @@ def __str__(self):
110110
return str(MessageToJson(self.to_proto()))
111111

112112
def __hash__(self):
113-
return hash((self.name))
113+
return hash(self.name)
114114

115115
def __getitem__(self, item):
116116
assert isinstance(item, list)
117117

118-
referenced_features = []
119-
for feature in self.features:
120-
if feature.name in item:
121-
referenced_features.append(feature)
122-
123118
cp = self.__copy__()
124-
cp.projection.features = referenced_features
119+
if self.features:
120+
referenced_features = []
121+
for feature in self.features:
122+
if feature.name in item:
123+
referenced_features.append(feature)
124+
cp.projection.features = referenced_features
125+
else:
126+
cp.projection.desired_features = item
125127

126128
return cp
127129

‎sdk/python/feast/feature_service.py‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,22 +102,36 @@ def __init__(
102102
self.created_timestamp = None
103103
self.last_updated_timestamp = None
104104
self.logging_config = logging_config
105-
self.infer_features()
105+
for feature_grouping in self._features:
106+
if isinstance(feature_grouping, BaseFeatureView):
107+
self.feature_view_projections.append(feature_grouping.projection)
106108

107109
def infer_features(self, fvs_to_update: Optional[Dict[str, FeatureView]] = None):
108-
self.feature_view_projections = []
109110
for feature_grouping in self._features:
110111
if isinstance(feature_grouping, BaseFeatureView):
111112
# For feature services that depend on an unspecified feature view, apply inferred schema
112-
if (
113-
fvs_to_update is not None
114-
and len(feature_grouping.projection.features) == 0
115-
and feature_grouping.name in fvs_to_update
116-
):
117-
feature_grouping.projection.features = fvs_to_update[
118-
feature_grouping.name
119-
].features
120-
self.feature_view_projections.append(feature_grouping.projection)
113+
if fvs_to_update and feature_grouping.name in fvs_to_update:
114+
if feature_grouping.projection.desired_features:
115+
desired_features = set(
116+
feature_grouping.projection.desired_features
117+
)
118+
actual_features = set(
119+
[
120+
f.name
121+
for f in fvs_to_update[feature_grouping.name].features
122+
]
123+
)
124+
assert desired_features.issubset(actual_features)
125+
# We need to set the features for the projection at this point so we ensure we're starting with
126+
# an empty list.
127+
feature_grouping.projection.features = []
128+
for f in fvs_to_update[feature_grouping.name].features:
129+
if f.name in desired_features:
130+
feature_grouping.projection.features.append(f)
131+
else:
132+
feature_grouping.projection.features = fvs_to_update[
133+
feature_grouping.name
134+
].features
121135
else:
122136
raise ValueError(
123137
f"The feature service {self.name} has been provided with an invalid type "

‎sdk/python/feast/feature_view_projection.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class FeatureViewProjection:
2727

2828
name: str
2929
name_alias: Optional[str]
30+
desired_features: List[str]
3031
features: List[Field]
3132
join_key_map: Dict[str, str] = {}
3233

@@ -51,6 +52,7 @@ def from_proto(proto: FeatureViewProjectionProto):
5152
name_alias=proto.feature_view_name_alias,
5253
features=[],
5354
join_key_map=dict(proto.join_key_map),
55+
desired_features=[],
5456
)
5557
for feature_column in proto.feature_columns:
5658
feature_view_projection.features.append(Field.from_proto(feature_column))
@@ -63,6 +65,7 @@ def from_definition(base_feature_view: "BaseFeatureView"):
6365
name=base_feature_view.name,
6466
name_alias=None,
6567
features=base_feature_view.features,
68+
desired_features=[],
6669
)
6770

6871
def get_feature(self, feature_name: str) -> Field:

‎sdk/python/feast/field.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ def __init__(
5050
self.tags = tags or {}
5151

5252
def __eq__(self, other):
53+
if type(self) != type(other):
54+
return False
55+
5356
if (
5457
self.name != other.name
5558
or self.dtype != other.dtype

‎sdk/python/feast/inference.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def update_data_sources_with_inferred_event_timestamp_col(
2727
data_source = data_source.batch_source
2828
if data_source.timestamp_field is None or data_source.timestamp_field == "":
2929
# prepare right match pattern for data source
30-
ts_column_type_regex_pattern = ""
30+
ts_column_type_regex_pattern: str
3131
# TODO(adchia): Move Spark source inference out of this logic
3232
if (
3333
isinstance(data_source, FileSource)

‎sdk/python/tests/integration/registration/test_inference.py‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -407,20 +407,37 @@ def test_update_feature_services_with_inferred_features(simple_dataset_1):
407407
feature_view_1 = FeatureView(
408408
name="test1", entities=[entity1], source=file_source,
409409
)
410-
feature_service = FeatureService(name="fs_1", features=[feature_view_1])
411-
assert len(feature_service.feature_view_projections) == 1
410+
feature_view_2 = FeatureView(
411+
name="test2", entities=[entity1], source=file_source,
412+
)
413+
414+
feature_service = FeatureService(
415+
name="fs_1", features=[feature_view_1[["string_col"]], feature_view_2]
416+
)
417+
assert len(feature_service.feature_view_projections) == 2
412418
assert len(feature_service.feature_view_projections[0].features) == 0
419+
assert len(feature_service.feature_view_projections[0].desired_features) == 1
420+
assert len(feature_service.feature_view_projections[1].features) == 0
421+
assert len(feature_service.feature_view_projections[1].desired_features) == 0
413422

414423
update_feature_views_with_inferred_features_and_entities(
415-
[feature_view_1], [entity1], RepoConfig(provider="local", project="test")
424+
[feature_view_1, feature_view_2],
425+
[entity1],
426+
RepoConfig(provider="local", project="test"),
416427
)
417428
feature_service.infer_features(
418-
fvs_to_update={feature_view_1.name: feature_view_1}
429+
fvs_to_update={
430+
feature_view_1.name: feature_view_1,
431+
feature_view_2.name: feature_view_2,
432+
}
419433
)
420434

421435
assert len(feature_view_1.schema) == 0
422436
assert len(feature_view_1.features) == 3
423-
assert len(feature_service.feature_view_projections[0].features) == 3
437+
assert len(feature_view_2.schema) == 0
438+
assert len(feature_view_2.features) == 3
439+
assert len(feature_service.feature_view_projections[0].features) == 1
440+
assert len(feature_service.feature_view_projections[1].features) == 3
424441

425442

426443
# TODO(felixwang9817): Add tests that interact with field mapping.

0 commit comments

Comments
 (0)