Skip to content

Commit

Permalink
fix: Update feature view APIs to prefer keyword args (#2472)
Browse files Browse the repository at this point in the history
* fix: Update feature view APIs to prefer keyword args

Signed-off-by: Achal Shah <achals@gmail.com>

* cr

Signed-off-by: Achal Shah <achals@gmail.com>

* fix tests

Signed-off-by: Achal Shah <achals@gmail.com>
  • Loading branch information
achals committed Mar 31, 2022
1 parent 83a11c6 commit 7c19cf7
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 25 deletions.
11 changes: 8 additions & 3 deletions sdk/python/feast/base_feature_view.py
Expand Up @@ -52,8 +52,9 @@ class BaseFeatureView(ABC):
@abstractmethod
def __init__(
self,
name: str,
features: List[Feature],
*,
name: Optional[str] = None,
features: Optional[List[Feature]] = None,
description: str = "",
tags: Optional[Dict[str, str]] = None,
owner: str = "",
Expand All @@ -72,8 +73,12 @@ def __init__(
Raises:
ValueError: A field mapping conflicts with an Entity or a Feature.
"""
if not name:
raise ValueError("Name needs to be provided")
self.name = name
self.features = features

self.features = features or []

self.description = description
self.tags = tags or {}
self.owner = owner
Expand Down
82 changes: 61 additions & 21 deletions sdk/python/feast/feature_view.py
Expand Up @@ -74,7 +74,7 @@ class FeatureView(BaseFeatureView):

name: str
entities: List[str]
ttl: timedelta
ttl: Optional[timedelta]
batch_source: DataSource
stream_source: Optional[DataSource]
features: List[Feature]
Expand All @@ -87,9 +87,10 @@ class FeatureView(BaseFeatureView):
@log_exceptions
def __init__(
self,
name: str,
entities: List[str],
ttl: Union[Duration, timedelta],
*args,
name: Optional[str] = None,
entities: Optional[List[str]] = None,
ttl: Optional[Union[Duration, timedelta]] = None,
batch_source: Optional[DataSource] = None,
stream_source: Optional[DataSource] = None,
features: Optional[List[Feature]] = None,
Expand Down Expand Up @@ -121,6 +122,54 @@ def __init__(
Raises:
ValueError: A field mapping conflicts with an Entity or a Feature.
"""

positional_attributes = ["name, entities, ttl"]

_name = name
_entities = entities
_ttl = ttl

if args:
warnings.warn(
(
"feature view parameters should be specified as a keyword argument instead of a positional arg."
"Feast 0.23+ will not support positional arguments to construct feature views"
),
DeprecationWarning,
)
if len(args) > len(positional_attributes):
raise ValueError(
f"Only {', '.join(positional_attributes)} are allowed as positional args when defining "
f"feature views, for backwards compatibility."
)
if len(args) >= 1:
_name = args[0]
if len(args) >= 2:
_entities = args[1]
if len(args) >= 3:
_ttl = args[2]

if not _name:
raise ValueError("feature view name needs to be specified")

self.name = _name
self.entities = _entities if _entities else [DUMMY_ENTITY_NAME]

if isinstance(_ttl, Duration):
self.ttl = timedelta(seconds=int(_ttl.seconds))
warnings.warn(
(
"The option to pass a Duration object to the ttl parameter is being deprecated. "
"Please pass a timedelta object instead. Feast 0.21 and onwards will not support "
"Duration objects."
),
DeprecationWarning,
)
elif isinstance(_ttl, timedelta) or _ttl is None:
self.ttl = _ttl
else:
raise ValueError(f"unknown value type specified for ttl {type(_ttl)}")

_features = features or []

if stream_source is not None and isinstance(stream_source, PushSource):
Expand All @@ -138,7 +187,7 @@ def __init__(
)
self.batch_source = batch_source

cols = [entity for entity in entities] + [feat.name for feat in _features]
cols = [entity for entity in self.entities] + [feat.name for feat in _features]
for col in cols:
if (
self.batch_source.field_mapping is not None
Expand All @@ -150,22 +199,13 @@ def __init__(
f"Entity or Feature name."
)

super().__init__(name, _features, description, tags, owner)
self.entities = entities if entities else [DUMMY_ENTITY_NAME]

if isinstance(ttl, Duration):
self.ttl = timedelta(seconds=int(ttl.seconds))
warnings.warn(
(
"The option to pass a Duration object to the ttl parameter is being deprecated. "
"Please pass a timedelta object instead. Feast 0.21 and onwards will not support "
"Duration objects."
),
DeprecationWarning,
)
else:
self.ttl = ttl

super().__init__(
name=name,
features=_features,
description=description,
tags=tags,
owner=owner,
)
self.online = online
self.stream_source = stream_source
self.materialization_intervals = []
Expand Down
8 changes: 7 additions & 1 deletion sdk/python/feast/on_demand_feature_view.py
Expand Up @@ -101,7 +101,13 @@ def __init__(
owner (optional): The owner of the on demand feature view, typically the email
of the primary maintainer.
"""
super().__init__(name, features, description, tags, owner)
super().__init__(
name=name,
features=features,
description=description,
tags=tags,
owner=owner,
)
if inputs and sources:
raise ValueError("At most one of `sources` or `inputs` can be specified.")
elif inputs:
Expand Down

0 comments on commit 7c19cf7

Please sign in to comment.