Skip to content

Commit a007be3

Browse files
fix
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
1 parent d4adcd5 commit a007be3

3 files changed

Lines changed: 85 additions & 46 deletions

File tree

sdk/python/feast/utils.py

Lines changed: 70 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -499,11 +499,11 @@ def _group_feature_refs(
499499
# on demand view to on demand view proto
500500
on_demand_view_index: Dict[str, "OnDemandFeatureView"] = {}
501501
for view in all_on_demand_feature_views:
502-
if view.projection and not getattr(view, "write_to_online_store", True):
503-
on_demand_view_index[view.projection.name_to_use()] = view
504-
elif view.projection and getattr(view, "write_to_online_store", True):
502+
if view.projection and getattr(view, "write_to_online_store", False):
505503
# we insert the ODFV view to FVs for ones that are written to the online store
506504
view_index[view.projection.name_to_use()] = view
505+
elif view.projection:
506+
on_demand_view_index[view.projection.name_to_use()] = view
507507

508508
# view name to feature names
509509
views_features = defaultdict(set)
@@ -1300,49 +1300,78 @@ def _get_feature_views_to_use(
13001300
hasattr(fv, "feature_transformation")
13011301
and fv.feature_transformation is not None
13021302
):
1303-
# Handle unified FeatureViews with transformations by finding the generated OnDemandFeatureView
1304-
try:
1305-
# Look for the auto-generated OnDemandFeatureView for online serving
1306-
online_fv_name = f"{fv.name}_online"
1307-
online_fv = registry.get_on_demand_feature_view(
1308-
online_fv_name, project, allow_cache
1309-
)
1310-
od_fvs_to_use.append(
1311-
online_fv.with_projection(copy.copy(projection))
1312-
if projection
1313-
else online_fv
1314-
)
1315-
except Exception:
1316-
# Fallback to the original FeatureView if auto-generated ODFV not found
1303+
# Check if this FeatureView requires on-demand transformation or
1304+
# if transformation happens during materialization.
1305+
#
1306+
# On-demand transformation is needed when:
1307+
# - FeatureView has source_views (depends on other FeatureViews for input)
1308+
#
1309+
# Materialization-time transformation (no on-demand needed) when:
1310+
# - FeatureView has a batch_source (DataSource) and online=True
1311+
# - Features are already transformed and stored in online store
1312+
has_source_views = hasattr(fv, "source_views") and fv.source_views
1313+
has_batch_source = hasattr(fv, "batch_source") and fv.batch_source
1314+
is_online_enabled = getattr(fv, "online", False)
1315+
1316+
# If transformation happens during materialization, treat as regular FV
1317+
if has_batch_source and is_online_enabled and not has_source_views:
1318+
# Features are already transformed and stored in online store
1319+
if (
1320+
hide_dummy_entity
1321+
and fv.entities # type: ignore[attr-defined]
1322+
and fv.entities[0] == DUMMY_ENTITY_NAME # type: ignore[attr-defined]
1323+
):
1324+
fv.entities = [] # type: ignore[attr-defined]
1325+
fv.entity_columns = [] # type: ignore[attr-defined]
13171326
fvs_to_use.append(
13181327
fv.with_projection(copy.copy(projection)) if projection else fv
13191328
)
1320-
1321-
# For unified FeatureViews, source FeatureViews are stored in source_views property
1322-
source_views = (
1323-
fv.source_views
1324-
if hasattr(fv, "source_views") and fv.source_views
1325-
else []
1326-
)
1327-
for source_fv in source_views:
1328-
# source_fv is already a FeatureView object for unified FeatureViews
1329-
if hasattr(source_fv, "name"):
1330-
# If it's a FeatureView, get it from registry to ensure it's up to date
1331-
source_fv = registry.get_any_feature_view(
1332-
source_fv.name, project, allow_cache
1329+
else:
1330+
# Handle unified FeatureViews with transformations that need on-demand transformation
1331+
# by finding the generated OnDemandFeatureView
1332+
try:
1333+
# Look for the auto-generated OnDemandFeatureView for online serving
1334+
online_fv_name = f"{fv.name}_online"
1335+
online_fv = registry.get_on_demand_feature_view(
1336+
online_fv_name, project, allow_cache
13331337
)
1334-
# TODO better way to handler dummy entities
1335-
if (
1336-
hide_dummy_entity
1337-
and source_fv.entities # type: ignore[attr-defined]
1338-
and source_fv.entities[0] == DUMMY_ENTITY_NAME # type: ignore[attr-defined]
1339-
):
1340-
source_fv.entities = [] # type: ignore[attr-defined]
1341-
source_fv.entity_columns = [] # type: ignore[attr-defined]
1338+
od_fvs_to_use.append(
1339+
online_fv.with_projection(copy.copy(projection))
1340+
if projection
1341+
else online_fv
1342+
)
1343+
except Exception:
1344+
# Fallback to the original FeatureView if auto-generated ODFV not found
1345+
fvs_to_use.append(
1346+
fv.with_projection(copy.copy(projection)) if projection else fv
1347+
)
1348+
1349+
# For unified FeatureViews with on-demand transformation,
1350+
# source FeatureViews need to be added to fetch input data
1351+
source_views = (
1352+
fv.source_views
1353+
if hasattr(fv, "source_views") and fv.source_views
1354+
else []
1355+
)
1356+
for source_fv in source_views:
1357+
# source_fv is already a FeatureView object for unified FeatureViews
1358+
if hasattr(source_fv, "name"):
1359+
# If it's a FeatureView, get it from registry to ensure it's up to date
1360+
source_fv = registry.get_any_feature_view(
1361+
source_fv.name, project, allow_cache
1362+
)
1363+
# TODO better way to handler dummy entities
1364+
if (
1365+
hide_dummy_entity
1366+
and source_fv.entities # type: ignore[attr-defined]
1367+
and source_fv.entities[0] == DUMMY_ENTITY_NAME # type: ignore[attr-defined]
1368+
):
1369+
source_fv.entities = [] # type: ignore[attr-defined]
1370+
source_fv.entity_columns = [] # type: ignore[attr-defined]
13421371

1343-
if source_fv not in fvs_to_use:
1344-
# For unified FeatureViews, add source views without complex projection handling
1345-
fvs_to_use.append(source_fv)
1372+
if source_fv not in fvs_to_use:
1373+
# For unified FeatureViews, add source views without complex projection handling
1374+
fvs_to_use.append(source_fv)
13461375
else:
13471376
if (
13481377
hide_dummy_entity

sdk/python/tests/unit/test_unified_pandas_transformation.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,13 @@ def all_types_transform(inputs: pd.DataFrame) -> pd.DataFrame:
260260
],
261261
).to_df()
262262

263-
# Verify the transformations
264-
assert (
265-
online_response["float32_output"].iloc[0]
266-
== online_response["conv_rate"].iloc[0] + 1.0
263+
# Verify the transformations (use np.isclose for floating-point comparisons)
264+
import numpy as np
265+
266+
assert np.isclose(
267+
online_response["float32_output"].iloc[0],
268+
online_response["conv_rate"].iloc[0] + 1.0,
269+
rtol=1e-6,
267270
)
268271
assert (
269272
online_response["string_output"].iloc[0]

sdk/python/tests/unit/test_unified_python_transformation.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,7 +751,14 @@ def test_python_transformation_returning_all_data_types(self):
751751
[rank for rank in expected_ranks if rank != "Locked"][-1:] or ["None"]
752752
)[0]
753753

754-
assert result["conv_rate_plus_acc"] == result["conv_rate"] + result["acc_rate"]
754+
# Use np.isclose for floating-point comparisons to handle precision issues
755+
import numpy as np
756+
757+
assert np.isclose(
758+
result["conv_rate_plus_acc"],
759+
result["conv_rate"] + result["acc_rate"],
760+
rtol=1e-6,
761+
)
755762
assert result["avg_daily_trips_plus_one"] == result["avg_daily_trips"] + 1
756763
assert result["highest_achieved_rank"] == highest_rank
757764
assert result["is_highest_rank"] == (expected_ranks[-1] != "Locked")

0 commit comments

Comments
 (0)