Skip to content

fix(core): don't blank a datetime column when its format coerces every value to NaT#42405

Open
rusackas wants to merge 1 commit into
masterfrom
fix-samples-mismatched-dttm-format
Open

fix(core): don't blank a datetime column when its format coerces every value to NaT#42405
rusackas wants to merge 1 commit into
masterfrom
fix-samples-mismatched-dttm-format

Conversation

@rusackas

Copy link
Copy Markdown
Member

SUMMARY

When a temporal column's declared python_date_format doesn't match its actual data, pd.to_datetime(..., errors="coerce") returns all-NaT, and _process_datetime_column assigned that result unconditionally — silently nulling the entire column.

This surfaces in the Samples / drill-detail pane: a chart whose granularity is a temporal column can send that column through normalization with a format that no longer fits the value it receives. Concretely, video_game_sales.year is an epoch-millis-backed column that inherits a %Y string format; pd.to_datetime(1136073600000, format="%Y")NaT, so every year value in the Samples tab renders as N/A, even though the chart itself renders (its SQL path passes the already-epoch value straight through). Any time-series chart on a dataset with a misfit temporal-column format hits this.

The fix: when a format coerces every non-null value to NaT, treat it as a mismatch and keep the original values rather than blanking the column. A format that successfully parses any value is applied exactly as before, so no currently-working case changes.

BEFORE/AFTER

  • Before: the whole column becomes NaT → Samples shows N/A for every row.
  • After: the original values are kept (a warning is logged), so the column renders.

TESTING INSTRUCTIONS

pytest tests/unit_tests/utils/test_core.py -k normalize_dttm — the new test_normalize_dttm_col_mismatched_format_keeps_values fails before the fix (column blanked to NaT) and passes after; the existing normalize tests are unchanged.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI (the Samples pane rendering)
  • Introduces new feature or API
  • Removes existing feature or API

🤖 Generated with Claude Code

…y value to NaT

When a temporal column's declared python_date_format doesn't match its
actual data, pd.to_datetime(..., errors='coerce') returns all-NaT and the
result was assigned unconditionally, silently nulling the whole column.
This surfaces in the Samples/drill-detail pane, where a chart's
granularity column (e.g. an epoch-millis 'year' that inherited a '%Y'
string format) renders as N/A for every row. Keep the original values
when a format coerces every non-null entry to NaT.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bito-code-review

bito-code-review Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #db7743

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 339d6ca..339d6ca
    • superset/utils/core.py
    • tests/unit_tests/utils/test_core.py
  • Files skipped - 0
  • Tools
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.24%. Comparing base (8c37d62) to head (339d6ca).

Files with missing lines Patch % Lines
superset/utils/core.py 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #42405      +/-   ##
==========================================
- Coverage   65.24%   65.24%   -0.01%     
==========================================
  Files        2795     2795              
  Lines      157643   157646       +3     
  Branches    36061    36062       +1     
==========================================
+ Hits       102853   102854       +1     
- Misses      52814    52815       +1     
- Partials     1976     1977       +1     
Flag Coverage Δ
hive 38.39% <0.00%> (-0.01%) ⬇️
mysql 57.57% <50.00%> (-0.01%) ⬇️
postgres 57.61% <50.00%> (-0.01%) ⬇️
presto 40.31% <0.00%> (-0.01%) ⬇️
python 59.02% <50.00%> (-0.01%) ⬇️
sqlite 57.24% <50.00%> (-0.01%) ⬇️
unit 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants