Skip to content

fix: Update feature view APIs to prefer keyword args #2472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
feast-ci-bot merged 3 commits into from
Mar 31, 2022
Merged

fix: Update feature view APIs to prefer keyword args #2472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2166e1
fix: Update feature view APIs to prefer keyword args
achals Mar 30, 2022
c27cf09
cr
achals Mar 31, 2022
e5b86be
fix tests
achals Mar 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions sdk/python/feast/base_feature_view.py
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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