Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed FVProjection 'name_to_use' field to 'name_alias' and changed '.set_projection' in FeatureView to ".with_projection". Also adjustments for some edge cases #1929

Merged
merged 1 commit into from
Oct 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion protos/feast/core/FeatureViewProjection.proto
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ message FeatureViewProjection {
string feature_view_name = 1;

// Alias for feature view name
string feature_view_name_to_use = 3;
string feature_view_name_alias = 3;

// The features of the feature view that are a part of the feature reference.
repeated FeatureSpecV2 feature_columns = 2;
Expand Down
32 changes: 22 additions & 10 deletions sdk/python/feast/feature_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 os
import warnings
from collections import Counter, OrderedDict, defaultdict
Expand Down Expand Up @@ -329,7 +330,10 @@ def _get_features(
)
for projection in feature_service_from_registry.feature_view_projections:
_feature_refs.extend(
[f"{projection.name_to_use}:{f.name}" for f in projection.features]
[
f"{projection.name_to_use()}:{f.name}"
for f in projection.features
]
)
else:
assert isinstance(_features, list)
Expand Down Expand Up @@ -939,7 +943,7 @@ def _populate_result_rows_from_feature_view(
if feature_data is None:
for feature_name in requested_features:
feature_ref = (
f"{table.projection.name_to_use}__{feature_name}"
f"{table.projection.name_to_use()}__{feature_name}"
if full_feature_names
else feature_name
)
Expand All @@ -949,7 +953,7 @@ def _populate_result_rows_from_feature_view(
else:
for feature_name in feature_data:
feature_ref = (
f"{table.projection.name_to_use}__{feature_name}"
f"{table.projection.name_to_use()}__{feature_name}"
if full_feature_names
else feature_name
)
Expand Down Expand Up @@ -1009,7 +1013,7 @@ def _augment_response_with_on_demand_transforms(

for transformed_feature in selected_subset:
transformed_feature_name = (
f"{odfv.projection.name_to_use}__{transformed_feature}"
f"{odfv.projection.name_to_use()}__{transformed_feature}"
if full_feature_names
else transformed_feature
)
Expand Down Expand Up @@ -1041,23 +1045,31 @@ def _get_feature_views_to_use(
)
}

fvs_to_use, od_fvs_to_use = [], []
if isinstance(features, FeatureService):
for fv_name, projection in {
projection.name: projection
for fv_name, projection in [
(projection.name, projection)
for projection in features.feature_view_projections
}.items():
]:
if fv_name in fvs:
fvs[fv_name].set_projection(projection)
fvs_to_use.append(
fvs[fv_name].with_projection(copy.copy(projection))
)
elif fv_name in od_fvs:
od_fvs[fv_name].set_projection(projection)
od_fvs_to_use.append(
od_fvs[fv_name].with_projection(copy.copy(projection))
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes made b/c of the possibility of having multiple same feature views with different projections

else:
raise ValueError(
f"The provided feature service {features.name} contains a reference to a feature view"
f"{fv_name} which doesn't exist. Please make sure that you have created the feature view"
f'{fv_name} and that you have registered it by running "apply".'
)
views_to_use = (fvs_to_use, od_fvs_to_use)
else:
views_to_use = ([*fvs.values()], [*od_fvs.values()])

return [*fvs.values()], [*od_fvs.values()]
return views_to_use

@log_exceptions_and_usage
def serve(self, port: int) -> None:
Expand Down
100 changes: 56 additions & 44 deletions sdk/python/feast/feature_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,21 @@ def __str__(self):
def __hash__(self):
return hash((id(self), self.name))

def __copy__(self):
fv = FeatureView(
name=self.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,
)
fv.projection = copy.copy(self.projection)
return fv

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

Expand All @@ -163,7 +178,8 @@ def __getitem__(self, item):
if feature.name in item:
referenced_features.append(feature)

self.projection.features = referenced_features
cp = self.__copy__()
cp.projection.features = referenced_features

return self

Expand Down Expand Up @@ -207,30 +223,54 @@ def is_valid(self):

def with_name(self, name: str):
"""
Produces a copy of this FeatureView with the passed name.
Renames this feature view by returning a copy of this feature view with an alias
set for the feature view name. This rename operation is only used as part of query
operations and will not modify the underlying FeatureView.

Args:
name: Name to assign to the FeatureView copy.

Returns:
A copy of this FeatureView with the name replaced with the 'name' input.
"""
fv = FeatureView(
name=self.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,
)
cp = self.__copy__()
cp.projection.name_alias = name

fv.set_projection(copy.copy(self.projection))
fv.projection.name_to_use = name
return cp

return fv
def with_projection(self, feature_view_projection: FeatureViewProjection):
"""
Sets the feature view projection by returning a copy of this feature view
with its projection set to the given projection. A projection is an
object that stores the modifications to a feature view that is used during
query operations.

Args:
feature_view_projection: The FeatureViewProjection object to link to this
OnDemandFeatureView.

Returns:
A copy of this FeatureView with its projection replaced with the 'feature_view_projection'
argument.
"""
if feature_view_projection.name != self.name:
raise ValueError(
f"The projection for the {self.name} FeatureView cannot be applied because it differs in name. "
f"The projection is named {feature_view_projection.name} and the name indicates which "
"FeatureView the projection is for."
)

for feature in feature_view_projection.features:
if feature not in self.features:
raise ValueError(
f"The projection for {self.name} cannot be applied because it contains {feature.name} which the "
"FeatureView doesn't have."
)

cp = self.__copy__()
cp.projection = feature_view_projection

return cp

def to_proto(self) -> FeatureViewProto:
"""
Expand Down Expand Up @@ -416,31 +456,3 @@ def infer_features_from_batch_source(self, config: RepoConfig):
"FeatureView",
f"Could not infer Features for the FeatureView named {self.name}.",
)

def set_projection(self, feature_view_projection: FeatureViewProjection) -> None:
"""
Setter for the projection object held by this FeatureView. A projection is an
object that stores the modifications to a FeatureView that is applied to the FeatureView
when the FeatureView is used such as during feature_store.get_historical_features.
This method also performs checks to ensure the projection is consistent with this
FeatureView before doing the set.

Args:
feature_view_projection: The FeatureViewProjection object to set this FeatureView's
'projection' field to.
"""
if feature_view_projection.name != self.name:
raise ValueError(
f"The projection for the {self.name} FeatureView cannot be applied because it differs in name. "
f"The projection is named {feature_view_projection.name} and the name indicates which "
"FeatureView the projection is for."
)

for feature in feature_view_projection.features:
if feature not in self.features:
raise ValueError(
f"The projection for {self.name} cannot be applied because it contains {feature.name} which the "
"FeatureView doesn't have."
)

self.projection = feature_view_projection
13 changes: 8 additions & 5 deletions sdk/python/feast/feature_view_projection.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import List
from typing import List, Optional

from attr import dataclass

Expand All @@ -11,12 +11,15 @@
@dataclass
class FeatureViewProjection:
name: str
name_to_use: str
name_alias: Optional[str]
features: List[Feature]

def name_to_use(self):
return self.name_alias or self.name

def to_proto(self):
feature_reference_proto = FeatureViewProjectionProto(
feature_view_name=self.name, feature_view_name_to_use=self.name_to_use
feature_view_name=self.name, feature_view_name_alias=self.name_alias
)
for feature in self.features:
feature_reference_proto.feature_columns.append(feature.to_proto())
Expand All @@ -27,7 +30,7 @@ def to_proto(self):
def from_proto(proto: FeatureViewProjectionProto):
ref = FeatureViewProjection(
name=proto.feature_view_name,
name_to_use=proto.feature_view_name_to_use,
name_alias=proto.feature_view_name_alias,
features=[],
)
for feature_column in proto.feature_columns:
Expand All @@ -39,6 +42,6 @@ def from_proto(proto: FeatureViewProjectionProto):
def from_definition(feature_grouping):
return FeatureViewProjection(
name=feature_grouping.name,
name_to_use=feature_grouping.name,
name_alias=None,
features=feature_grouping.features,
)
2 changes: 1 addition & 1 deletion sdk/python/feast/infra/offline_stores/offline_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.name_to_use,
name=feature_view.projection.name_to_use(),
ttl=ttl_seconds,
entities=join_keys,
features=features,
Expand Down
4 changes: 2 additions & 2 deletions sdk/python/feast/infra/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ def _get_requested_feature_views_to_features_dict(

found = False
for fv in feature_views:
if fv.projection.name_to_use == 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.name_to_use == 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)

Expand Down
Loading