Skip to content

Fix Polars temporal column stats JSON serialization for tracker - #1675

Open
dgvj-work wants to merge 2 commits into
apache:mainfrom
dgvj-work:fix/polars-temporal-stats-json-safe
Open

Fix Polars temporal column stats JSON serialization for tracker#1675
dgvj-work wants to merge 2 commits into
apache:mainfrom
dgvj-work:fix/polars-temporal-stats-json-safe

Conversation

@dgvj-work

@dgvj-work dgvj-work commented Aug 1, 2026

Copy link
Copy Markdown

Make Polars Date/Datetime column stats JSON-safe for the tracker, and keep Time/Duration off the datetime UI path.

Changes

How I tested this

  • pytest ui/sdk/tests/tracking/test_polars_col_stats.py ui/sdk/tests/tracking/test_polars_stats.py (22 passed)
  • ruff format on the touched files (passed)

Notes

Closes #1127

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

@dgvj-work
dgvj-work force-pushed the fix/polars-temporal-stats-json-safe branch 2 times, most recently from a65d947 to 670e8f4 Compare August 1, 2026 05:45
Datetime tracking for apache#1127 already avoids std(); extend the same
datetime stats path so Time and Duration values serialize cleanly,
and add a Datetime regression test matching the reported failure.

@jernejfrank jernejfrank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, just a small thing since not everything renders on UI

Comment on lines +134 to +143
def _temporal_to_jsonable(value: object) -> object:
# datetime.datetime is a subclass of datetime.date. Time/Duration also arrive via
# selectors.temporal() and must be JSON-safe for the UI (same base_data_type="datetime").
if isinstance(value, (datetime.date, datetime.time)):
return value.isoformat()
if isinstance(value, datetime.timedelta):
return str(value)
return value


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks a couple of daterendering on the UI:

"2021-01-01T00:00:00" → valid Date
  "01:00:00"             → Invalid Date / NaN
  "1 day, 0:00:00"       → Invalid Date / NaN
  "2 days, 0:00:00"      → Invalid Date / NaN

Can we route Time/Duration to unhandled stats for now?

@dgvj-work dgvj-work Aug 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jernejfrank Good catch, those Time/Duration strings are not valid JS Dates, so they were breaking the datetime range renderer.

I routed Time and Duration to unhandled stats instead of datetime stats. Date/Datetime still go through the datetime path with ISO timestamps (2021-01-01T00:00:00), which new Date(...) can parse.

The #1127 Datetime regression coverage is still there, plus a test that Time/Duration stay on the unhandled path (no min/max date strings).

@dgvj-work
dgvj-work force-pushed the fix/polars-temporal-stats-json-safe branch from 4fe0f3f to e423a0d Compare August 24, 2026 03:06
Time and Duration values are not valid JS Dates, so sending them through datetime stats breaks the UI range renderer. Keep Date/Datetime on the ISO datetime path and leave Time/Duration for unhandled stats.
@dgvj-work
dgvj-work force-pushed the fix/polars-temporal-stats-json-safe branch from e423a0d to 463ffc6 Compare August 24, 2026 03:06
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.

Hamilton tracker gives an error with datetime columns in polars dataframes

2 participants