Skip to content

Commit

Permalink
Added .with_name method in FeatureView/OnDemandFeatureView classes fo…
Browse files Browse the repository at this point in the history
…r name aliasing. FeatureViewProjection will hold this information (feast-dev#1872)

* Added .with_name method in FV with test case

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* Correctly sort imports

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* correct lint errors

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* Modified FeatureNameCollisionError msg

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* fixed error msg test

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* updated code to preserve original FV name in base_name field

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* Updated proto conversion methods and fixed failures

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* fixed proto numberings

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* wip

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* Cleaned code up

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* added copy method in FVProjections

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>

* use python's copy lib rather than creating new copy method.

Signed-off-by: David Y Liu <davidyliuliu@gmail.com>
  • Loading branch information
mavysavydav authored Oct 7, 2021
1 parent a5b8c26 commit 66fbb51
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 39 deletions.
3 changes: 3 additions & 0 deletions protos/feast/core/FeatureViewProjection.proto
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +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;
}
5 changes: 3 additions & 2 deletions sdk/python/feast/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
22 changes: 11 additions & 11 deletions sdk/python/feast/feature_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,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.name_to_use}:{f.name}" for f in projection.features]
)
else:
assert isinstance(_features, list)
Expand Down Expand Up @@ -905,7 +905,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(
Expand Down Expand Up @@ -935,7 +939,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.name_to_use}__{feature_name}"
if full_feature_names
else feature_name
)
Expand All @@ -945,7 +949,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.name_to_use}__{feature_name}"
if full_feature_names
else feature_name
)
Expand All @@ -972,16 +976,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:
Expand Down Expand Up @@ -1009,7 +1009,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.name_to_use}__{transformed_feature}"
if full_feature_names
else transformed_feature
)
Expand Down
28 changes: 28 additions & 0 deletions sdk/python/feast/feature_view.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 re
import warnings
from datetime import datetime, timedelta
Expand Down Expand Up @@ -204,6 +205,33 @@ 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.
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,
)

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

return fv

def to_proto(self) -> FeatureViewProto:
"""
Converts a feature view object to its protobuf representation.
Expand Down
13 changes: 10 additions & 3 deletions sdk/python/feast/feature_view_projection.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
@dataclass
class FeatureViewProjection:
name: str
name_to_use: str
features: List[Feature]

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())
Expand All @@ -24,7 +25,11 @@ 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_to_use=proto.feature_view_name_to_use,
features=[],
)
for feature_column in proto.feature_columns:
ref.features.append(Feature.from_proto(feature_column))

Expand All @@ -33,5 +38,7 @@ 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_to_use=feature_grouping.name,
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.name,
name=feature_view.projection.name_to_use,
ttl=ttl_seconds,
entities=join_keys,
features=features,
Expand Down
16 changes: 6 additions & 10 deletions sdk/python/feast/infra/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.name_to_use == 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.name_to_use == 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}")
Expand Down
20 changes: 20 additions & 0 deletions sdk/python/feast/on_demand_feature_view.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import copy
import functools
from types import MethodType
from typing import Dict, List, Union, cast
Expand Down Expand Up @@ -68,6 +69,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(copy.copy(self.projection))
odfv.projection.name_to_use = name

return odfv

def to_proto(self) -> OnDemandFeatureViewProto:
"""
Converts an on demand feature view object to its protobuf representation.
Expand Down
25 changes: 13 additions & 12 deletions sdk/python/tests/unit/test_feature_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,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:
Expand All @@ -43,9 +43,10 @@ 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

0 comments on commit 66fbb51

Please # to comment.