-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat(rust,python): Implement max/min methods for dtypes #19494
Conversation
4af49f1
to
1ac7f8c
Compare
UInt64 => Scalar::from(u64::MAX), | ||
Float32 => Scalar::from(f32::INFINITY), | ||
Float64 => Scalar::from(f64::INFINITY), | ||
dt => polars_bail!(ComputeError: "cannot determine upper bound for dtype `{}`", dt), |
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.
I think here we can use something like NaiveDate::MAX()
and MIN
from chrono here to get date limits, and likewise for NaiveDateTime
. As an example, the naive date max value is 95026236
and we can see we hit that limit here as well:
import polars as pl
pl.Series([95026236]).cast(pl.Date) # works
pl.Series([95026237]).cast(pl.Date) # PanicException: out-of-range date
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.
Good point. Perhaps could be discussed in a follow-up issue?
I don't know if we should include chrono limitations here or not.
Nice one; we should probably think about deprecating |
2bab756
to
6128f8e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19494 +/- ##
=======================================
Coverage 79.83% 79.84%
=======================================
Files 1536 1536
Lines 211357 211384 +27
Branches 2445 2445
=======================================
+ Hits 168744 168769 +25
- Misses 42058 42060 +2
Partials 555 555 ☔ View full report in Codecov by Sentry. |
This can be merged? |
Close #19412