Make entity description optional and fix empty table_ref (feast-dev#1425)

jklegar · web-flow · commit ae477ddecc82 · 2021-03-30T18:25:39.000-07:00
* Make entity description optional and fix empty table_ref

Signed-off-by: Jacob Klegar &lt;jacob@tecton.ai&gt;

* Remove empty descriptions

Signed-off-by: Jacob Klegar &lt;jacob@tecton.ai&gt;

* fix lint

Signed-off-by: Jacob Klegar &lt;jacob@tecton.ai&gt;
diff --git a/sdk/python/feast/data_source.py b/sdk/python/feast/data_source.py
@@ -676,7 +676,7 @@ def to_proto(self) -> DataSourceProto:
 
     def get_table_query_string(self) -> str:
         """Returns a string that can directly be used to reference this table in SQL"""
-        if self.table_ref is not None:
+        if self.table_ref:
             return f"`{self.table_ref}`"
         else:
             return f"({self.query})"
diff --git a/sdk/python/feast/entity.py b/sdk/python/feast/entity.py
@@ -34,8 +34,8 @@ class Entity:
     def __init__(
         self,
         name: str,
-        description: str,
         value_type: ValueType,
+        description: str = "",
         labels: Optional[MutableMapping[str, str]] = None,
     ):
         self._name = name
diff --git a/sdk/python/feast/offline_store.py b/sdk/python/feast/offline_store.py
@@ -125,10 +125,7 @@ def pull_latest_from_table_or_query(
         end_date: datetime,
     ) -> pyarrow.Table:
         assert isinstance(data_source, BigQuerySource)
-        if data_source.table_ref:
-            from_expression = f"`{data_source.table_ref}`"
-        else:
-            from_expression = f"({data_source.query})"
+        from_expression = data_source.get_table_query_string()
 
         partition_by_entity_string = ", ".join(entity_names)
         if partition_by_entity_string != "":
diff --git a/sdk/python/tests/online_write_benchmark.py b/sdk/python/tests/online_write_benchmark.py
@@ -59,7 +59,7 @@ def benchmark_writes():
         # This is just to set data source to something, we're not reading from parquet source here.
         parquet_path = os.path.join(temp_dir, "data.parquet")
 
-        driver = Entity(name="driver_id", value_type=ValueType.INT64, description="")
+        driver = Entity(name="driver_id", value_type=ValueType.INT64)
         table = create_driver_hourly_stats_feature_view(
             create_driver_hourly_stats_source(parquet_path=parquet_path)
         )
diff --git a/sdk/python/tests/test_entity.py b/sdk/python/tests/test_entity.py
@@ -84,3 +84,7 @@ def test_entity_without_labels_empty_dict():
     entity = Entity("my-entity", description="My entity", value_type=ValueType.STRING)
     assert entity.labels == dict()
     assert len(entity.labels) == 0
+
+
+def test_entity_without_description():
+    Entity("my-entity", value_type=ValueType.STRING)
diff --git a/sdk/python/tests/test_historical_retrieval.py b/sdk/python/tests/test_historical_retrieval.py
@@ -224,8 +224,8 @@ def test_historical_features_from_parquet_sources():
             temp_dir, customer_df
         )
         customer_fv = create_customer_daily_profile_feature_view(customer_source)
-        driver = Entity(name="driver", value_type=ValueType.INT64, description="")
-        customer = Entity(name="customer", value_type=ValueType.INT64, description="")
+        driver = Entity(name="driver", value_type=ValueType.INT64)
+        customer = Entity(name="customer", value_type=ValueType.INT64)
 
         store = FeatureStore(
             config=RepoConfig(
@@ -326,8 +326,8 @@ def test_historical_features_from_bigquery_sources():
         )
         customer_fv = create_customer_daily_profile_feature_view(customer_source)
 
-        driver = Entity(name="driver", value_type=ValueType.INT64, description="")
-        customer = Entity(name="customer", value_type=ValueType.INT64, description="")
+        driver = Entity(name="driver", value_type=ValueType.INT64)
+        customer = Entity(name="customer", value_type=ValueType.INT64)
 
         store = FeatureStore(
             config=RepoConfig(

Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,7 @@ def benchmark_writes():`
`59`	`59`	`# This is just to set data source to something, we're not reading from parquet source here.`
`60`	`60`	`parquet_path = os.path.join(temp_dir, "data.parquet")`
`61`	`61`
`62`		`- driver = Entity(name="driver_id", value_type=ValueType.INT64, description="")`
	`62`	`+ driver = Entity(name="driver_id", value_type=ValueType.INT64)`
`63`	`63`	`table = create_driver_hourly_stats_feature_view(`
`64`	`64`	`create_driver_hourly_stats_source(parquet_path=parquet_path)`
`65`	`65`	`)`