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

perf(rust, python): parse time zones outside of downcast_iter() in replace_time_zone #10713

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

MarcoGorelli
Copy link
Collaborator

@MarcoGorelli MarcoGorelli commented Aug 24, 2023

Noticed while looking into #10090

I probably introduced this inefficiency the first time round, sorry about that

@github-actions github-actions bot added performance Performance issues or improvements python Related to Python Polars rust Related to Rust Polars labels Aug 24, 2023
@MarcoGorelli MarcoGorelli changed the title perf(rust, python): parse time zones outside of downcast_iter() in re… perf(rust, python): parse time zones outside of downcast_iter() in replace_time_zone Aug 24, 2023
@MarcoGorelli MarcoGorelli marked this pull request as ready for review August 24, 2023 11:46
@orlp
Copy link
Collaborator

orlp commented Aug 24, 2023

I think this can be written quite a bit cleaner than the current nested match (didn't test it):

fn parse_timezone(s: &str) -> PolarsResult<Tz> {
    s.parse().map_err(|e| polars_err!(ComputeError: "unable to parse time zone '{s}': {e}"))
}

pub fn replace_time_zone(
    ca: &DatetimeChunked,
    time_zone: Option<&str>,
    use_earliest: Option<bool>,
) -> PolarsResult<DatetimeChunked> {
    let from = parse_timezone(ca.time_zone().as_deref().unwrap_or("UTC"))?;
    let to = parse_timezone(time_zone.unwrap_or("UTC"))?;
    let chunks = ca.downcast_iter().map(|arr| {
        replace_time_zone_kernel(arr, ca.time_unit().to_arrow(), &from, &to, use_earliest)
    });
    let ca = ChunkedArray::try_from_chunk_iter(ca.name(), chunks)?;
    let mut out = ca.into_datetime(ca.time_unit(), time_zone.map(|x| x.to_string()));
    out.set_sorted_flag(ca.is_sorted_flag());
    Ok(out)
}

The question mark is your friend :)

@MarcoGorelli
Copy link
Collaborator Author

much cleaner, thanks 🙏

@ritchie46 ritchie46 merged commit 21e8cf0 into pola-rs:main Aug 24, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance issues or improvements python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants