refactor: Delete unnecessary errors.go file (feast-dev#2392)

felixwang9817 · web-flow · commit a8144ddf9ca7 · 2022-03-10T11:46:25.000-08:00
* Clean up docstrings

Signed-off-by: Felix Wang &lt;wangfelix98@gmail.com&gt;

* Move featureNameCollisionError from errors.go into featurestore.go

Signed-off-by: Felix Wang &lt;wangfelix98@gmail.com&gt;
diff --git a/go/internal/feast/errors.go b/go/internal/feast/errors.go
diff --git a/go/internal/feast/featurestore.go b/go/internal/feast/featurestore.go
@@ -401,52 +401,51 @@ func (fs *FeatureStore) validateEntityValues(joinKeyValues map[string]*types.Rep
 }
 
 func (fs *FeatureStore) validateFeatureRefs(featureRefs []string, fullFeatureNames bool) error {
-	collidedFeatureRefs := make(map[string]int)
+	featureRefCounter := make(map[string]int)
 	if fullFeatureNames {
 		for _, featureRef := range featureRefs {
-			collidedFeatureRefs[featureRef] += 1
+			featureRefCounter[featureRef]++
 		}
-		for featureName, occurrences := range collidedFeatureRefs {
+		for featureName, occurrences := range featureRefCounter {
 			if occurrences == 1 {
-				delete(collidedFeatureRefs, featureName)
+				delete(featureRefCounter, featureName)
 			}
 		}
-		if len(collidedFeatureRefs) >= 1 {
-			collidedFeatureRefList := make([]string, len(collidedFeatureRefs))
+		if len(featureRefCounter) >= 1 {
+			collidedFeatureRefs := make([]string, len(featureRefCounter))
 			index := 0
-			for featureName := range collidedFeatureRefs {
-				collidedFeatureRefList[index] = featureName
-				index += 1
+			for collidedFeatureRef := range featureRefCounter {
+				collidedFeatureRefs[index] = collidedFeatureRef
+				index++
 			}
-			return NewFeatureNameCollisionError(collidedFeatureRefList, fullFeatureNames)
+			return featureNameCollisionError{collidedFeatureRefs, fullFeatureNames}
 		}
 	} else {
 		for _, featureRef := range featureRefs {
 			_, featureName, err := parseFeatureReference(featureRef)
 			if err != nil {
 				return err
 			}
-			collidedFeatureRefs[featureName] += 1
+			featureRefCounter[featureName]++
 		}
-
-		for featureName, occurrences := range collidedFeatureRefs {
+		for featureName, occurrences := range featureRefCounter {
 			if occurrences == 1 {
-				delete(collidedFeatureRefs, featureName)
+				delete(featureRefCounter, featureName)
 			}
 		}
-		if len(collidedFeatureRefs) >= 1 {
-			collidedFeatureRefList := make([]string, 0)
+		if len(featureRefCounter) >= 1 {
+			collidedFeatureRefs := make([]string, 0)
 			for _, featureRef := range featureRefs {
 				_, featureName, err := parseFeatureReference(featureRef)
 				if err != nil {
 					return err
 				}
-				if _, ok := collidedFeatureRefs[featureName]; ok {
-					collidedFeatureRefList = append(collidedFeatureRefList, featureRef)
+				if _, ok := featureRefCounter[featureName]; ok {
+					collidedFeatureRefs = append(collidedFeatureRefs, featureRef)
 				}
 
 			}
-			return errors.New(fmt.Sprintf("featureNameCollisionError: %s; %t", strings.Join(collidedFeatureRefList, ", "), fullFeatureNames))
+			return featureNameCollisionError{collidedFeatureRefs, fullFeatureNames}
 		}
 	}
 	return nil
@@ -858,3 +857,12 @@ func getFeatureResponseMeta(featureNameAlias string, featureName string, fullFea
 		return featureName
 	}
 }
+
+type featureNameCollisionError struct {
+	featureRefCollisions []string
+	fullFeatureNames     bool
+}
+
+func (e featureNameCollisionError) Error() string {
+	return fmt.Sprintf("featureNameCollisionError: %s; %t", strings.Join(e.featureRefCollisions, ", "), e.fullFeatureNames)
+}
diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py
@@ -784,12 +784,12 @@ def get_historical_features(
                 columns (e.g., customer_id, driver_id) on which features need to be joined, as well as a event_timestamp
                 column used to ensure point-in-time correctness. Either a Pandas DataFrame can be provided or a string
                 SQL query. The query must be of a format supported by the configured offline store (e.g., BigQuery)
-            features: A list of features, that should be retrieved from the offline store.
-                Either a list of string feature references can be provided or a FeatureService object.
-                Feature references are of the format "feature_view:feature", e.g., "customer_fv:daily_transactions".
-            full_feature_names: A boolean that provides the option to add the feature view prefixes to the feature names,
-                changing them from the format "feature" to "feature_view__feature" (e.g., "daily_transactions" changes to
-                "customer_fv__daily_transactions"). By default, this value is set to False.
+            features: The list of features that should be retrieved from the offline store. These features can be
+                specified either as a list of string feature references or as a feature service. String feature
+                references must have format "feature_view:feature", e.g. "customer_fv:daily_transactions".
+            full_feature_names: If True, feature names will be prefixed with the corresponding feature view name,
+                changing them from the format "feature" to "feature_view__feature" (e.g. "daily_transactions"
+                changes to "customer_fv__daily_transactions").
 
         Returns:
             RetrievalJob which can be used to materialize the results.
@@ -822,7 +822,6 @@ def get_historical_features(
             ... )
             >>> feature_data = retrieval_job.to_df()
         """
-
         _feature_refs = self._get_features(features)
         (
             all_feature_views,
@@ -1194,12 +1193,13 @@ def get_online_features(
         infinity (cache forever).
 
         Args:
-            features: List of feature references that will be returned for each entity.
-                Each feature reference should have the following format:
-                "feature_view:feature" where "feature_view" & "feature" refer to
-                the Feature and FeatureView names respectively.
-                Only the feature name is required.
+            features: The list of features that should be retrieved from the online store. These features can be
+                specified either as a list of string feature references or as a feature service. String feature
+                references must have format "feature_view:feature", e.g. "customer_fv:daily_transactions".
             entity_rows: A list of dictionaries where each key-value is an entity-name, entity-value pair.
+            full_feature_names: If True, feature names will be prefixed with the corresponding feature view name,
+                changing them from the format "feature" to "feature_view__feature" (e.g. "daily_transactions"
+                changes to "customer_fv__daily_transactions").
 
         Returns:
             OnlineResponse containing the feature data in records.
@@ -1870,16 +1870,26 @@ def _validate_entity_values(join_key_values: Dict[str, List[Value]]):
 
 
 def _validate_feature_refs(feature_refs: List[str], full_feature_names: bool = False):
+    """
+    Validates that there are no collisions among the feature references.
+
+    Args:
+        feature_refs: List of feature references to validate. Feature references must have format
+            "feature_view:feature", e.g. "customer_fv:daily_transactions".
+        full_feature_names: If True, the full feature references are compared for collisions; if False,
+            only the feature names are compared.
+
+    Raises:
+        FeatureNameCollisionError: There is a collision among the feature references.
+    """
     collided_feature_refs = []
 
     if full_feature_names:
         collided_feature_refs = [
             ref for ref, occurrences in Counter(feature_refs).items() if occurrences > 1
         ]
     else:
-        feature_names = [
-            ref.split(":")[1] if ":" in ref else ref for ref in feature_refs
-        ]
+        feature_names = [ref.split(":")[1] for ref in feature_refs]
         collided_feature_names = [
             ref
             for ref, occurrences in Counter(feature_names).items()

Original file line number	Diff line number	Diff line change
`@@ -401,52 +401,51 @@ func (fs FeatureStore) validateEntityValues(joinKeyValues map[string]types.Rep`
`401`	`401`	`}`
`402`	`402`
`403`	`403`	`func (fs *FeatureStore) validateFeatureRefs(featureRefs []string, fullFeatureNames bool) error {`
`404`		`- collidedFeatureRefs := make(map[string]int)`
	`404`	`+ featureRefCounter := make(map[string]int)`
`405`	`405`	`if fullFeatureNames {`
`406`	`406`	`for _, featureRef := range featureRefs {`
`407`		`- collidedFeatureRefs[featureRef] += 1`
	`407`	`+ featureRefCounter[featureRef]++`
`408`	`408`	`}`
`409`		`- for featureName, occurrences := range collidedFeatureRefs {`
	`409`	`+ for featureName, occurrences := range featureRefCounter {`
`410`	`410`	`if occurrences == 1 {`
`411`		`- delete(collidedFeatureRefs, featureName)`
	`411`	`+ delete(featureRefCounter, featureName)`
`412`	`412`	`}`
`413`	`413`	`}`
`414`		`- if len(collidedFeatureRefs) >= 1 {`
`415`		`- collidedFeatureRefList := make([]string, len(collidedFeatureRefs))`
	`414`	`+ if len(featureRefCounter) >= 1 {`
	`415`	`+ collidedFeatureRefs := make([]string, len(featureRefCounter))`
`416`	`416`	`index := 0`
`417`		`- for featureName := range collidedFeatureRefs {`
`418`		`- collidedFeatureRefList[index] = featureName`
`419`		`- index += 1`
	`417`	`+ for collidedFeatureRef := range featureRefCounter {`
	`418`	`+ collidedFeatureRefs[index] = collidedFeatureRef`
	`419`	`+ index++`
`420`	`420`	`}`
`421`		`- return NewFeatureNameCollisionError(collidedFeatureRefList, fullFeatureNames)`
	`421`	`+ return featureNameCollisionError{collidedFeatureRefs, fullFeatureNames}`
`422`	`422`	`}`
`423`	`423`	`} else {`
`424`	`424`	`for _, featureRef := range featureRefs {`
`425`	`425`	`_, featureName, err := parseFeatureReference(featureRef)`
`426`	`426`	`if err != nil {`
`427`	`427`	`return err`
`428`	`428`	`}`
`429`		`- collidedFeatureRefs[featureName] += 1`
	`429`	`+ featureRefCounter[featureName]++`
`430`	`430`	`}`
`431`		`-`
`432`		`- for featureName, occurrences := range collidedFeatureRefs {`
	`431`	`+ for featureName, occurrences := range featureRefCounter {`
`433`	`432`	`if occurrences == 1 {`
`434`		`- delete(collidedFeatureRefs, featureName)`
	`433`	`+ delete(featureRefCounter, featureName)`
`435`	`434`	`}`
`436`	`435`	`}`
`437`		`- if len(collidedFeatureRefs) >= 1 {`
`438`		`- collidedFeatureRefList := make([]string, 0)`
	`436`	`+ if len(featureRefCounter) >= 1 {`
	`437`	`+ collidedFeatureRefs := make([]string, 0)`
`439`	`438`	`for _, featureRef := range featureRefs {`
`440`	`439`	`_, featureName, err := parseFeatureReference(featureRef)`
`441`	`440`	`if err != nil {`
`442`	`441`	`return err`
`443`	`442`	`}`
`444`		`- if _, ok := collidedFeatureRefs[featureName]; ok {`
`445`		`- collidedFeatureRefList = append(collidedFeatureRefList, featureRef)`
	`443`	`+ if _, ok := featureRefCounter[featureName]; ok {`
	`444`	`+ collidedFeatureRefs = append(collidedFeatureRefs, featureRef)`
`446`	`445`	`}`
`447`	`446`
`448`	`447`	`}`
`449`		`- return errors.New(fmt.Sprintf("featureNameCollisionError: %s; %t", strings.Join(collidedFeatureRefList, ", "), fullFeatureNames))`
	`448`	`+ return featureNameCollisionError{collidedFeatureRefs, fullFeatureNames}`
`450`	`449`	`}`
`451`	`450`	`}`
`452`	`451`	`return nil`
`@@ -858,3 +857,12 @@ func getFeatureResponseMeta(featureNameAlias string, featureName string, fullFea`
`858`	`857`	`return featureName`
`859`	`858`	`}`
`860`	`859`	`}`
	`860`	`+`
	`861`	`+type featureNameCollisionError struct {`
	`862`	`+ featureRefCollisions []string`
	`863`	`+ fullFeatureNames bool`
	`864`	`+}`
	`865`	`+`
	`866`	`+func (e featureNameCollisionError) Error() string {`
	`867`	`+ return fmt.Sprintf("featureNameCollisionError: %s; %t", strings.Join(e.featureRefCollisions, ", "), e.fullFeatureNames)`
	`868`	`+}`