-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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: More Int128
testing and related fixes
#20494
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20494 +/- ##
==========================================
- Coverage 79.02% 79.02% -0.01%
==========================================
Files 1563 1563
Lines 220557 220609 +52
Branches 2492 2502 +10
==========================================
+ Hits 174300 174335 +35
- Misses 45684 45700 +16
- Partials 573 574 +1 ☔ View full report in Codecov by Sentry. |
crates/polars-python/src/map/mod.rs
Outdated
@@ -28,6 +28,7 @@ impl PyArrowPrimitiveType for Int8Type {} | |||
impl PyArrowPrimitiveType for Int16Type {} | |||
impl PyArrowPrimitiveType for Int32Type {} | |||
impl PyArrowPrimitiveType for Int64Type {} | |||
impl PyArrowPrimitiveType for Int128Type {} |
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.
This is maybe a misnomer, as Pyarrow doesn't support i128 primitive type.
Looking at this code, I also don't understand why we have this trait. Maybe we can remove it and replace the PyArrowPrimitiveType
with PolarsNumericType
.
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.
Yeah, I think PyArrowPrimitiveType
is a misnomer here. I renamed this to PyPolarsNumericType
.
I tried replacing it entirely with PolarsNumericType
as you suggested but couldn't figure out how to get around this build error:
error[E0119]: conflicting implementations of trait `ApplyLambda<'_>` for type `polars::prelude::ChunkedArray<polars::prelude::BooleanType>`
--> crates/polars-python/src/map/series.rs:571:1
|
280 | impl<'a> ApplyLambda<'a> for BooleanChunked {
| ------------------------------------------- first implementation here
...
571 | / impl<'a, T> ApplyLambda<'a> for ChunkedArray<T>
572 | | where
573 | | T: PolarsNumericType,
574 | | T::Native: IntoPyObject<'a> + FromPyObject<'a>,
575 | | ChunkedArray<T>: IntoSeries,
| |________________________________^ conflicting implementation for `polars::prelude::ChunkedArray<polars::prelude::BooleanType>`
|
= note: upstream crates may add a new impl of trait `polars::prelude::PolarsNumericType` for type `polars::prelude::BooleanType` in future versions
It appears there is a similar trait PolarsOpsNumericType
used in a similar way here:
trait PolarsOpsNumericType: PolarsNumericType {} |
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.
Ah, there we have the reason for this new trait. :)
Again, looks great. Thanks a lot @lukemanley |
No description provided.