fix: Make name a keyword arg for the Entity class (feast-dev#2467)

achals · web-flow · commit 43847dea74b2 · 2022-03-30T12:49:39.000-07:00
* fix: Make name a kwarg for Entity

Signed-off-by: Achal Shah &lt;achals@gmail.com&gt;

* merge

Signed-off-by: Achal Shah &lt;achals@gmail.com&gt;

* update test

Signed-off-by: Achal Shah &lt;achals@gmail.com&gt;
diff --git a/go/internal/feast/ondemandfeatureview.go b/go/internal/feast/ondemandfeatureview.go
@@ -6,7 +6,7 @@ import (
 )
 
 type OnDemandFeatureView struct {
-	base                        *BaseFeatureView
+	base                         *BaseFeatureView
 	sourceFeatureViewProjections map[string]*FeatureViewProjection
 	sourceRequestDataSources     map[string]*core.DataSource_RequestDataOptions
 }
diff --git a/sdk/python/feast/entity.py b/sdk/python/feast/entity.py
@@ -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 warnings
 from datetime import datetime
 from typing import Dict, Optional
 
@@ -53,17 +54,35 @@ class Entity:
     @log_exceptions
     def __init__(
         self,
-        name: str,
+        *args,
+        name: Optional[str] = None,
         value_type: ValueType = ValueType.UNKNOWN,
         description: str = "",
         join_key: Optional[str] = None,
         tags: Optional[Dict[str, str]] = None,
         owner: str = "",
     ):
         """Creates an Entity object."""
-        self.name = name
+        if len(args) == 1:
+            warnings.warn(
+                (
+                    "Entity name should be specified as a keyword argument instead of a positional arg."
+                    "Feast 0.23+ will not support positional arguments to construct Entities"
+                ),
+                DeprecationWarning,
+            )
+        if len(args) > 1:
+            raise ValueError(
+                "All arguments to construct an entity should be specified as keyword arguments only"
+            )
+
+        self.name = args[0] if len(args) > 0 else name
+
+        if not self.name:
+            raise ValueError("Name needs to be specified")
+
         self.value_type = value_type
-        self.join_key = join_key if join_key else name
+        self.join_key = join_key if join_key else self.name
         self.description = description
         self.tags = tags if tags is not None else {}
         self.owner = owner
diff --git a/sdk/python/tests/unit/test_entity.py b/sdk/python/tests/unit/test_entity.py
@@ -11,32 +11,55 @@
 # 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 assertpy
+import pytest
 
 from feast.entity import Entity
 from feast.value_type import ValueType
 
 
 def test_join_key_default():
-    entity = Entity("my-entity", description="My entity", value_type=ValueType.STRING)
+    with pytest.deprecated_call():
+        entity = Entity(
+            "my-entity", description="My entity", value_type=ValueType.STRING
+        )
     assert entity.join_key == "my-entity"
 
 
 def test_entity_class_contains_tags():
-    entity = Entity(
-        "my-entity",
-        description="My entity",
-        value_type=ValueType.STRING,
-        tags={"key1": "val1", "key2": "val2"},
-    )
+    with pytest.deprecated_call():
+        entity = Entity(
+            "my-entity",
+            description="My entity",
+            value_type=ValueType.STRING,
+            tags={"key1": "val1", "key2": "val2"},
+        )
     assert "key1" in entity.tags.keys() and entity.tags["key1"] == "val1"
     assert "key2" in entity.tags.keys() and entity.tags["key2"] == "val2"
 
 
 def test_entity_without_tags_empty_dict():
-    entity = Entity("my-entity", description="My entity", value_type=ValueType.STRING)
+    with pytest.deprecated_call():
+        entity = Entity(
+            "my-entity", description="My entity", value_type=ValueType.STRING
+        )
     assert entity.tags == dict()
     assert len(entity.tags) == 0
 
 
 def test_entity_without_description():
-    Entity("my-entity", value_type=ValueType.STRING)
+    with pytest.deprecated_call():
+        Entity("my-entity", value_type=ValueType.STRING)
+
+
+def test_name_not_specified():
+    assertpy.assert_that(lambda: Entity(value_type=ValueType.STRING)).raises(ValueError)
+
+
+def test_multiple_args():
+    assertpy.assert_that(lambda: Entity("a", ValueType.STRING)).raises(ValueError)
+
+
+def test_name_keyword(recwarn):
+    Entity(name="my-entity", value_type=ValueType.STRING)
+    assert len(recwarn) == 0

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ import (`
`6`	`6`	`)`
`7`	`7`
`8`	`8`	`type OnDemandFeatureView struct {`
`9`		`- base *BaseFeatureView`
	`9`	`+ base *BaseFeatureView`
`10`	`10`	`sourceFeatureViewProjections map[string]*FeatureViewProjection`
`11`	`11`	`sourceRequestDataSources map[string]*core.DataSource_RequestDataOptions`
`12`	`12`	`}`