made error logic/messages more descriptive

Signed-off-by: Mwad22 <51929507+Mwad22@users.noreply.github.com>
feast-dev · feast-ci-bot · Jul 8, 2021 · Jun 12, 2021 · Jun 16, 2021 · Jun 16, 2021
commit 082fca7169cf2440d82e6fa7fbe7019916214d83
@@ -90,11 +90,16 @@ def __init__(self, offline_store_name: str, data_source_name: str):
 
 class FeatureNameCollisionError(Exception):
     def __init__(self, feature_refs_collisions: List[str]):
-        feature_name_collisions = [ref.split(":")[1] for ref in feature_refs_collisions]
-        feature_names = ", ".join(x for x in feature_name_collisions)
+        collisions = [ref.split(":", 1) for ref in feature_refs_collisions]
+        collision_feature_views = [y for (y, _) in collisions]
+        collision_feature_names = [x for (_, x) in collisions]
+        feature_names = ", ".join(set(collision_feature_names))
+        feature_views = ", ".join(set(collision_feature_views))
         super().__init__(
             f"The following feature name(s) have collisions: {feature_names}. Set 'full_feature_names' "
-            f"argument in the data retrieval function to True to use the full feature name which is prefixed by the feature view name."
+            f"argument in the data retrieval function to True to use the full feature name which is prefixed "
+            f"by the feature view name, or rename colliding features.\nCollisions occur in the following "
+            f"feature view(s): {feature_views}."
         )
 
 

@@ -553,6 +553,7 @@ def get_online_features(
             project=self.project, allow_cache=True
         )
 
+        _validate_feature_refs(feature_refs, full_feature_names)
         grouped_refs = _group_feature_refs(feature_refs, all_feature_views)
         for table, requested_features in grouped_refs:
             entity_keys = _get_table_entity_keys(
@@ -613,13 +614,19 @@ def _entity_row_to_field_values(
 
 
 def _validate_feature_refs(feature_refs: List[str], full_feature_names: bool = False):
-    feature_names = [ref.split(":")[1] for ref in feature_refs]
-    feature_name, occurrences = Counter(feature_names).most_common(1)[0]
-    if count > 1:
+    if full_feature_names:
         collided_feature_refs = [
-            ref for ref in feature_refs if ref.endswith(":" + feature_name)
-        ]
-        raise FeatureNameCollisionError(collided_feature_refs)
+            ref for ref, occurrences in Counter(feature_refs).items() if occurrences > 1]
+        if len(collided_feature_refs) > 0:
+            raise FeatureNameCollisionError(collided_feature_refs)
+    else:
+        feature_names = [ref.split(":")[1] for ref in feature_refs]
+        feature_name, occurrences = Counter(feature_names).most_common(1)[0]
+        if occurrences > 1:
+            collided_feature_refs = [
+                ref for ref in feature_refs if ref.endswith(":" + feature_name)
+            ]
+            raise FeatureNameCollisionError(collided_feature_refs)
 
 
 def _group_feature_refs(

@@ -129,6 +129,7 @@ def get_historical_features(
         entity_df: Union[pandas.DataFrame, str],
         registry: Registry,
         project: str,
+        full_feature_names: bool = False,
     ) -> RetrievalJob:
         job = self.offline_store.get_historical_features(
             config=config,
@@ -137,5 +138,6 @@ def get_historical_features(
             entity_df=entity_df,
             registry=registry,
             project=project,
+            full_feature_names=full_feature_names,
         )
         return job
@@ -204,12 +204,22 @@ def get_expected_training_df(
     expected_df = expected_df[[event_timestamp] + current_cols]
 
     # Cast some columns to expected types, since we lose information when converting pandas DFs into Python objects.
-    expected_column_types = {
-        "order_is_success": "int32",
-        "driver_stats__conv_rate": "float32",
-        "customer_profile__current_balance": "float32",
-        "customer_profile__avg_passenger_count": "float32",
-    }
+
+    if full_feature_names:
+        expected_column_types = {
+            "order_is_success": "int32",
+            "driver_stats__conv_rate": "float32",
+            "customer_profile__current_balance": "float32",
+            "customer_profile__avg_passenger_count": "float32",
+        }
+    else:
+        expected_column_types = {
+            "order_is_success": "int32",
+            "conv_rate": "float32",
+            "current_balance": "float32",
+            "avg_passenger_count": "float32",
+        }
+
     for col, typ in expected_column_types.items():
         expected_df[col] = expected_df[col].astype(typ)
 

@@ -267,7 +267,6 @@ def check_offline_and_online_features(
             assert abs(df.to_dict()["value"][0] - expected_value) < 1e-6
         else:
             assert math.isnan(df.to_dict()["value"][0])
-
 
 
 def run_offline_online_store_consistency_test(
@@ -341,13 +340,15 @@ def test_redis_offline_online_store_consistency(full_feature_names: bool):
     with prep_redis_fs_and_fv() as (fs, fv):
         run_offline_online_store_consistency_test(fs, fv, full_feature_names)
 
+
 @pytest.mark.parametrize("full_feature_names", [True, False])
 @pytest.mark.integration
-def test_dynamodb_offline_online_store_consistency(full_feature_names:bool):
+def test_dynamodb_offline_online_store_consistency(full_feature_names: bool):
     with prep_dynamodb_fs_and_fv() as (fs, fv):
         run_offline_online_store_consistency_test(fs, fv, full_feature_names)
 
+
 @pytest.mark.parametrize("full_feature_names", [True, False])
-def test_local_offline_online_store_consistency(full_feature_names:bool):
+def test_local_offline_online_store_consistency(full_feature_names: bool):
     with prep_local_fs_and_fv() as (fs, fv):
         run_offline_online_store_consistency_test(fs, fv, full_feature_names)