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(rust, python): Implement missing extract conversion for Time datatype #5161

Merged
merged 1 commit into from
Oct 11, 2022
Merged

fix(rust, python): Implement missing extract conversion for Time datatype #5161

merged 1 commit into from
Oct 11, 2022

Conversation

alexander-beedie
Copy link
Collaborator

@alexander-beedie alexander-beedie commented Oct 11, 2022

Added missing extract conversion for Time, allowingFromPyObject to handle python times (in addition to already-supported datetimes/dates).

Example

from datetime import date, time
import polars as pl

# "for demonstration purposes only..." :)
midday = lambda x: time(12,0)

df = pl.DataFrame( 
    data = [date.today()],
    columns = [("today", pl.Datetime)],
).with_column( 
    pl.col("today").apply( midday ).alias("midday"),
)

Before: (panic with RuntimeError on unsupported type)

# pyo3_runtime.PanicException: called `Result::unwrap()` on an `Err` value:
# PyErr {
#   type: <class 'RuntimeError'>,
#   value: RuntimeError('BindingsError: "type not supported time(12, 0)"'),
#   traceback: None,
# }

After: (successful use/return of python time)

# ┌────────────┬──────────┐
# │ today      ┆ midday   │
# │ ---        ┆ ---      │
# │ date       ┆ time     │
# ╞════════════╪══════════╡
# │ 2022-10-11 ┆ 12:00:00 │
# └────────────┴──────────┘

Misc:

  • The typename matches in FromPyObject were done with contains; seems safer to use eq to avoid potential (if unlikely) mismatches, unless there's a reason not to?
  • Added object => pl.Object as a valid _PY_TYPE_TO_DTYPE lookup.

@github-actions github-actions bot added fix Bug fix rust Related to Rust Polars labels Oct 11, 2022
@alexander-beedie alexander-beedie changed the title fix(rust): Implement missing extract conversion for Time datatype fix(rust, python): Implement missing extract conversion for Time datatype Oct 11, 2022
@github-actions github-actions bot added the python Related to Python Polars label Oct 11, 2022
@ritchie46
Copy link
Member

That took a while to be discovered. Thanks!

@ritchie46 ritchie46 merged commit 1dd7578 into pola-rs:master Oct 11, 2022
@alexander-beedie alexander-beedie deleted the anyvalue-extract-time branch October 12, 2022 08:14
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