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

fix: Enable missing features in polars-time #15558

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

filabrazilska
Copy link
Contributor

Without having smartstring/serde compiling with --all-features fails because of the smartstring values not being serializable:

$ cargo build --all-features  -p polars-time
   Compiling polars-time v0.38.3 (/home/fila/workspace/polars/crates/polars-time)
error[E0277]: the trait bound `SmartString<LazyCompact>: rolling_window::_::_serde::Serialize` is not satisfied
    --> crates/polars-time/src/group_by/dynamic.rs:21:38
     |
21   |   #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
     |                                        ^^^^^^^^^ the trait `rolling_window::_::_serde::Serialize` is not implemented for `SmartString<LazyCompact>`
22   |   pub struct DynamicGroupOptions {
23   | /     /// Time or index column.
24   | |     pub index_column: SmartString,
     | |_________________________________- required by a bound introduced by this call
     |
     = help: the following other types implement trait `rolling_window::_::_serde::Serialize`:
               bool
               char
               isize
               i8
               i16
               i32
               i64
               i128
             and 134 others
note: required by a bound in `rolling_window::_::_serde::ser::SerializeStruct::serialize_field`
    --> /home/fila/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.197/src/ser/mod.rs:1865:12
     |
1859 |     fn serialize_field<T: ?Sized>(
     |        --------------- required by a bound in this associated function
...
1865 |         T: Serialize;
     |            ^^^^^^^^^ required by this bound in `SerializeStruct::serialize_field`
     = note: this error originates in the derive macro `Serialize` (in Nightly builds, run with -Z macro-backtrace for more info)

The abs feature on the polars-ops crate is necessary for the tests in polars-time to pass.

Without having `smartstring/serde` compiling with `--all-features`
fails because of the smartstring values not being serializable:

```
$ cargo build --all-features  -p polars-time
   Compiling polars-time v0.38.3 (/home/fila/workspace/polars/crates/polars-time)
error[E0277]: the trait bound `SmartString<LazyCompact>: rolling_window::_::_serde::Serialize` is not satisfied
    --> crates/polars-time/src/group_by/dynamic.rs:21:38
     |
21   |   #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
     |                                        ^^^^^^^^^ the trait `rolling_window::_::_serde::Serialize` is not implemented for `SmartString<LazyCompact>`
22   |   pub struct DynamicGroupOptions {
23   | /     /// Time or index column.
24   | |     pub index_column: SmartString,
     | |_________________________________- required by a bound introduced by this call
     |
     = help: the following other types implement trait `rolling_window::_::_serde::Serialize`:
               bool
               char
               isize
               i8
               i16
               i32
               i64
               i128
             and 134 others
note: required by a bound in `rolling_window::_::_serde::ser::SerializeStruct::serialize_field`
    --> /home/fila/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.197/src/ser/mod.rs:1865:12
     |
1859 |     fn serialize_field<T: ?Sized>(
     |        --------------- required by a bound in this associated function
...
1865 |         T: Serialize;
     |            ^^^^^^^^^ required by this bound in `SerializeStruct::serialize_field`
     = note: this error originates in the derive macro `Serialize` (in Nightly builds, run with -Z macro-backtrace for more info)

```

The `abs` feature on the `polars-ops` crate is necessary for the tests
in `polars-time` to pass.
Copy link

codspeed-hq bot commented Apr 9, 2024

CodSpeed Performance Report

Merging #15558 will not alter performance

Comparing filabrazilska:smartstring_serde (b4b5423) with main (42d3697)

Summary

✅ 22 untouched benchmarks

@filabrazilska filabrazilska changed the title Enable missing features in polars-time fix: Enable missing features in polars-time Apr 9, 2024
@github-actions github-actions bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Apr 9, 2024
Copy link
Member

@ritchie46 ritchie46 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good. Thanks!

@ritchie46 ritchie46 merged commit e72b868 into pola-rs:main Apr 10, 2024
20 checks passed
@filabrazilska filabrazilska deleted the smartstring_serde branch April 10, 2024 09:18
mbuhidar pushed a commit to mbuhidar/polars that referenced this pull request Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants