Skip to content

fix: Normalize Dask timestamps to UTC without a row-wise apply - #6818

Open
Daksha1611 wants to merge 1 commit into
feast-dev:masterfrom
Daksha1611:fix/dask-empty-entity-df
Open

fix: Normalize Dask timestamps to UTC without a row-wise apply#6818
Daksha1611 wants to merge 1 commit into
feast-dev:masterfrom
Daksha1611:fix/dask-empty-entity-df

Conversation

@Daksha1611

Copy link
Copy Markdown

What this PR does / why we need it:

_normalize_timestamp made timestamp columns tz-aware with a row-wise apply that
declared meta=(timestamp_field, "datetime64[ns, UTC]"). meta only declares the
dtype — the partition gets whatever the lambda returns. With zero rows the lambda never
runs, so the computed column stayed tz-naive while meta claimed UTC, and the tz-aware
comparison in _filter_ttl then raised:

TypeError: Invalid comparison between dtype=datetime64[ns] and DatetimeArray

A zero-row entity_df is a normal case in batch scoring — the upstream query matched
nothing for that run — so get_historical_features crashed instead of returning an
empty result.

While fixing this I found the same mismatch on a second path: a column carrying a
non-UTC timezone also diverged from the declared meta, because the lambda returned any
value that already had tzinfo untouched. That is covered by a test here too.

Both applies are replaced with dd.to_datetime(..., utc=True), which localizes tz-naive
values, converts tz-aware ones, and yields the correct dtype for an empty frame. It is
also vectorized, so it drops a Python-level call per row on non-empty frames.

Which issue(s) this PR fixes:

Fixes #6817

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

New sdk/python/tests/unit/infra/offline_stores/test_dask_empty_entity_df.py covers
normalization and TTL filtering at 0 and 1 rows, plus the non-UTC case. Three of its
five assertions fail on master and pass here; the two 1-row cases pass both ways,
pinning the non-empty behaviour as unchanged.

Verified end to end as well: get_historical_features(...).to_df() against a file
offline store with an otherwise identical entity frame raised TypeError at 0 rows
before this change, and now returns an empty frame with the expected columns while the
1-row case is unchanged.

sdk/python/tests/unit/infra/offline_stores plus test_unit_feature_store.py: 235
passed. The 20 errors in that run are MongoDB testcontainers failing to start locally
and are present on master too.

ruff check, ruff format --check and mypy are clean on both changed files.

Misc

Point-in-time correctness is unaffected — I re-ran the filter_by_created_timestamp
scenario (a row backfilled after the entity timestamp) and it still excludes the late
row with the flag and includes it without.

_normalize_timestamp made timestamp columns tz-aware with a row-wise apply
declaring meta="datetime64[ns, UTC]". meta only declares the dtype; the
partition gets whatever the lambda returns. With zero rows the lambda never
runs, so the computed column stayed tz-naive while meta claimed UTC, and the
tz-aware comparison in _filter_ttl then raised:

    TypeError: Invalid comparison between dtype=datetime64[ns] and DatetimeArray

A zero-row entity_df is a normal case in batch scoring, when the upstream
query matched nothing for that run, so get_historical_features crashed rather
than returning an empty result.

A column carrying a non-UTC timezone diverged from the declared meta the same
way, because the lambda returned any value that already had tzinfo untouched.

Replace both applies with dd.to_datetime(..., utc=True), which localizes
tz-naive values, converts tz-aware ones, and yields the correct dtype for an
empty frame. It is also vectorized, so it avoids a Python-level call per row
on non-empty frames.

Signed-off-by: Daksha1611 <mehtadaksha1611@gmail.com>
@Daksha1611
Daksha1611 requested a review from a team as a code owner September 6, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_historical_features raises TypeError on a zero-row entity_df (Dask/file offline store)

1 participant