fix: string-to-datetime prep transform fails on timestamps without seconds - #262
Open
iabaako wants to merge 1 commit into
Open
fix: string-to-datetime prep transform fails on timestamps without seconds#262iabaako wants to merge 1 commit into
iabaako wants to merge 1 commit into
Conversation
3 tasks
|
| self, mock_prep_apply, sample_polars_df | ||
| ): | ||
| """A failing transform shows st.error instead of crashing the app.""" | ||
| import datasure.views.prep_view as pv |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Pull Request Summary 🚀
What does this PR do? 📝
Fixes #261: converting a string column to datetime via a Prep step crashed the whole Streamlit app with an uncaught traceback whenever the value's time component had no seconds (e.g.
3/15/2026 17:28).H:MM/HH:MM) format variants alongside every existing date+time shape in_parse_flexible_datetime(ISO, Stata-style18aug2025,M/D/Y,D/M/Y), so timestamps missing seconds now parse correctly.prep_add_step→prep_apply_action) intry/except (ValidationError, OperationError)withst.error(...), matching the pattern already used by the correction view's interactive apply, so a still-unsupported format now shows a friendly error instead of crashing the whole app.Why is this change needed? 🤔
Timestamps without seconds are a common real-world shape (spreadsheet/Excel exports, some
submissiondate-like fields), and every format_parse_flexible_datetimetried requiredHH:MM:SS. On top of the parsing gap, the interactive prep-apply path had no error boundary, so any parse failure took down the whole app with a raw traceback rather than a usable message.How was this implemented? 🛠️
processing/prep.py: added a%H:%M-only validator/format pair immediately after each existing%H:%M:%Sentry inTransformColumnsOperation._parse_flexible_datetime'sformats_to_trylist. The no-seconds validators are anchored ($) so they can't accidentally match a with-seconds value.views/prep_view.py: wrappedprep_apply_action(project_id, label, PrepActionResult(**prep_args))inprep_add_step's Add-button handler intry/except, showingst.error(f"Error adding preparation step: {e!s}")on failure and only callingst.success/st.rerunon success.How to test or reproduce? 🧪
uv run python -m pytest— full suite passes.just lint-py/just fmt-python/ pre-commit — clean.3/15/2026 17:28— it now converts successfully instead of crashing the app.Screenshots (if applicable) 📷
N/A — bug fix, no new visual components.
Checklist ✅
Note on stacking
This PR is stacked on
fix/253-standardize-reapply-error-handling(#260), which is still an open draft. Base is set to that branch rather thanmainso review only shows this PR's incremental diff; once #260 merges, retarget this PR's base tomain.🤖 Generated with Claude Code