New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(python): Convert date and datetime in literal construction #16018
base: main
Are you sure you want to change the base?
Conversation
598ca60
to
22992c8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16018 +/- ##
==========================================
+ Coverage 80.75% 81.33% +0.57%
==========================================
Files 1393 1403 +10
Lines 179459 183308 +3849
Branches 2924 2928 +4
==========================================
+ Hits 144929 149089 +4160
+ Misses 34027 33716 -311
Partials 503 503 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you tweak the unit tests to include a wider range of dates/datetimes (including pre-epoch, etc)? 👍
@alexander-beedie thanks, working on parametrizing and indeed running into cases where we're outside the Datetime casting ability, but I'm not sure how to deal with it. It looks like from datetime import date
import polars as pl
pl.Series([date(1677, 9, 22)]).cast(pl.Datetime("ns")) # 1677-09-22 00:00:00
pl.Series([date(1677, 9, 21)]).cast(pl.Datetime("ns")) # 2262-04-11 23:34:33.709551616 |
22992c8
to
849292c
Compare
@alexander-beedie fixed up a few things and expanded the tests. Let me know if you think it needs more testing or changes. I restricted the times in the test to be within the Date -> Nanosecond allowance, as that can be addressed in the other issue I opened. |
849292c
to
8a8dd4e
Compare
Sorry, let this one sit; can you rebase/update and I'll take another look :) |
8a8dd4e
to
e717464
Compare
@alexander-beedie should be good once tests complete. |
Fixes #16012.