Skip to content
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

Rolling aggregations not working for various integer types #20511

Closed
2 tasks done
lukemanley opened this issue Dec 31, 2024 · 3 comments · Fixed by #20512
Closed
2 tasks done

Rolling aggregations not working for various integer types #20511

lukemanley opened this issue Dec 31, 2024 · 3 comments · Fixed by #20512
Labels
bug Something isn't working needs triage Awaiting prioritization by a maintainer python Related to Python Polars

Comments

@lukemanley
Copy link
Contributor

Checks

  • I have checked that this issue has not already been reported.
  • I have confirmed this bug exists on the latest version of Polars.

Reproducible example

import polars as pl

s = pl.Series("a", [1, 2, 3, 4], pl.Int8)
s.rolling_sum(2)

Log output

thread '<unnamed>' panicked at /Users/runner/work/polars/polars/crates/polars-time/src/chunkedarray/rolling_window/dispatch.rs:218:9:
not implemented for dtype Int8

Issue description

The example above panics for i8, i16, i128, u8, u16 - all of which are feature gated inside the macro below. The python example works as expected if I remove the feature gates from this macro and rebuild, though I'm not sure that is the correct fix?

#[macro_export]
macro_rules! with_match_physical_numeric_polars_type {(
$key_type:expr, | $_:tt $T:ident | $($body:tt)*
) => ({
macro_rules! __with_ty__ {( $_ $T:ident ) => ( $($body)* )}
use $crate::datatypes::DataType::*;
match $key_type {
#[cfg(feature = "dtype-i8")]
Int8 => __with_ty__! { Int8Type },
#[cfg(feature = "dtype-i16")]
Int16 => __with_ty__! { Int16Type },
Int32 => __with_ty__! { Int32Type },
Int64 => __with_ty__! { Int64Type },
#[cfg(feature = "dtype-i128")]
Int128 => __with_ty__! { Int128Type },
#[cfg(feature = "dtype-u8")]
UInt8 => __with_ty__! { UInt8Type },
#[cfg(feature = "dtype-u16")]
UInt16 => __with_ty__! { UInt16Type },
UInt32 => __with_ty__! { UInt32Type },
UInt64 => __with_ty__! { UInt64Type },
Float32 => __with_ty__! { Float32Type },
Float64 => __with_ty__! { Float64Type },
dt => panic!("not implemented for dtype {:?}", dt),
}
})}

Expected behavior

Works without panicking

Installed versions

--------Version info---------
Polars:              1.18.0
Index type:          UInt32
Platform:            macOS-14.6.1-arm64-arm-64bit
Python:              3.12.7 (main, Oct 26 2024, 21:27:43) [Clang 15.0.0 (clang-1500.3.9.4)]
LTS CPU:             False

----Optional dependencies----
adbc_driver_manager  <not installed>
altair               <not installed>
azure.identity       <not installed>
boto3                <not installed>
cloudpickle          <not installed>
connectorx           <not installed>
deltalake            <not installed>
fastexcel            <not installed>
fsspec               <not installed>
gevent               <not installed>
google.auth          <not installed>
great_tables         <not installed>
matplotlib           <not installed>
nest_asyncio         <not installed>
numpy                <not installed>
openpyxl             <not installed>
pandas               <not installed>
pyarrow              <not installed>
pydantic             <not installed>
pyiceberg            <not installed>
sqlalchemy           <not installed>
torch                <not installed>
xlsx2csv             <not installed>
xlsxwriter           <not installed>
@lukemanley lukemanley added bug Something isn't working needs triage Awaiting prioritization by a maintainer python Related to Python Polars labels Dec 31, 2024
@ritchie46
Copy link
Member

I suspect the feature gates aren't propagated properly to the polars-time crate.

@Chuck321123
Copy link

The same problem occurs for rolling_min and rolling_max

@lukemanley
Copy link
Contributor Author

The same problem occurs for rolling_min and rolling_max

yep, I think #20512 fixes (and tests) min/max/sum

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage Awaiting prioritization by a maintainer python Related to Python Polars
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants