Skip to content

Commit

Permalink
fix: Fix feature view __getitem__ for feature services (#2769)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
achals authored Jun 8, 2022
1 parent 8a39743 commit 88cc47d
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 24 deletions.
16 changes: 9 additions & 7 deletions sdk/python/feast/base_feature_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,20 @@ def __str__(self):
return str(MessageToJson(self.to_proto()))

def __hash__(self):
return hash((self.name))
return hash(self.name)

def __getitem__(self, item):
assert isinstance(item, list)

referenced_features = []
for feature in self.features:
if feature.name in item:
referenced_features.append(feature)

cp = self.__copy__()
cp.projection.features = referenced_features
if self.features:
referenced_features = []
for feature in self.features:
if feature.name in item:
referenced_features.append(feature)
cp.projection.features = referenced_features
else:
cp.projection.desired_features = item

return cp

Expand Down
36 changes: 25 additions & 11 deletions sdk/python/feast/feature_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,22 +102,36 @@ def __init__(
self.created_timestamp = None
self.last_updated_timestamp = None
self.logging_config = logging_config
self.infer_features()
for feature_grouping in self._features:
if isinstance(feature_grouping, BaseFeatureView):
self.feature_view_projections.append(feature_grouping.projection)

def infer_features(self, fvs_to_update: Optional[Dict[str, FeatureView]] = None):
self.feature_view_projections = []
for feature_grouping in self._features:
if isinstance(feature_grouping, BaseFeatureView):
# For feature services that depend on an unspecified feature view, apply inferred schema
if (
fvs_to_update is not None
and len(feature_grouping.projection.features) == 0
and feature_grouping.name in fvs_to_update
):
feature_grouping.projection.features = fvs_to_update[
feature_grouping.name
].features
self.feature_view_projections.append(feature_grouping.projection)
if fvs_to_update and feature_grouping.name in fvs_to_update:
if feature_grouping.projection.desired_features:
desired_features = set(
feature_grouping.projection.desired_features
)
actual_features = set(
[
f.name
for f in fvs_to_update[feature_grouping.name].features
]
)
assert desired_features.issubset(actual_features)
# We need to set the features for the projection at this point so we ensure we're starting with
# an empty list.
feature_grouping.projection.features = []
for f in fvs_to_update[feature_grouping.name].features:
if f.name in desired_features:
feature_grouping.projection.features.append(f)
else:
feature_grouping.projection.features = fvs_to_update[
feature_grouping.name
].features
else:
raise ValueError(
f"The feature service {self.name} has been provided with an invalid type "
Expand Down
3 changes: 3 additions & 0 deletions sdk/python/feast/feature_view_projection.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class FeatureViewProjection:

name: str
name_alias: Optional[str]
desired_features: List[str]
features: List[Field]
join_key_map: Dict[str, str] = {}

Expand All @@ -51,6 +52,7 @@ def from_proto(proto: FeatureViewProjectionProto):
name_alias=proto.feature_view_name_alias,
features=[],
join_key_map=dict(proto.join_key_map),
desired_features=[],
)
for feature_column in proto.feature_columns:
feature_view_projection.features.append(Field.from_proto(feature_column))
Expand All @@ -63,6 +65,7 @@ def from_definition(base_feature_view: "BaseFeatureView"):
name=base_feature_view.name,
name_alias=None,
features=base_feature_view.features,
desired_features=[],
)

def get_feature(self, feature_name: str) -> Field:
Expand Down
3 changes: 3 additions & 0 deletions sdk/python/feast/field.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ def __init__(
self.tags = tags or {}

def __eq__(self, other):
if type(self) != type(other):
return False

if (
self.name != other.name
or self.dtype != other.dtype
Expand Down
2 changes: 1 addition & 1 deletion sdk/python/feast/inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def update_data_sources_with_inferred_event_timestamp_col(
data_source = data_source.batch_source
if data_source.timestamp_field is None or data_source.timestamp_field == "":
# prepare right match pattern for data source
ts_column_type_regex_pattern = ""
ts_column_type_regex_pattern: str
# TODO(adchia): Move Spark source inference out of this logic
if (
isinstance(data_source, FileSource)
Expand Down
27 changes: 22 additions & 5 deletions sdk/python/tests/integration/registration/test_inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,20 +407,37 @@ def test_update_feature_services_with_inferred_features(simple_dataset_1):
feature_view_1 = FeatureView(
name="test1", entities=[entity1], source=file_source,
)
feature_service = FeatureService(name="fs_1", features=[feature_view_1])
assert len(feature_service.feature_view_projections) == 1
feature_view_2 = FeatureView(
name="test2", entities=[entity1], source=file_source,
)

feature_service = FeatureService(
name="fs_1", features=[feature_view_1[["string_col"]], feature_view_2]
)
assert len(feature_service.feature_view_projections) == 2
assert len(feature_service.feature_view_projections[0].features) == 0
assert len(feature_service.feature_view_projections[0].desired_features) == 1
assert len(feature_service.feature_view_projections[1].features) == 0
assert len(feature_service.feature_view_projections[1].desired_features) == 0

update_feature_views_with_inferred_features_and_entities(
[feature_view_1], [entity1], RepoConfig(provider="local", project="test")
[feature_view_1, feature_view_2],
[entity1],
RepoConfig(provider="local", project="test"),
)
feature_service.infer_features(
fvs_to_update={feature_view_1.name: feature_view_1}
fvs_to_update={
feature_view_1.name: feature_view_1,
feature_view_2.name: feature_view_2,
}
)

assert len(feature_view_1.schema) == 0
assert len(feature_view_1.features) == 3
assert len(feature_service.feature_view_projections[0].features) == 3
assert len(feature_view_2.schema) == 0
assert len(feature_view_2.features) == 3
assert len(feature_service.feature_view_projections[0].features) == 1
assert len(feature_service.feature_view_projections[1].features) == 3


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

0 comments on commit 88cc47d

Please sign in to comment.