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(python): Convert date and datetime in literal construction #16018

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mcrumiller
Copy link
Contributor

Fixes #16012.

@github-actions github-actions bot added fix Bug fix python Related to Python Polars labels May 2, 2024
@mcrumiller mcrumiller force-pushed the lit-date-datetime branch 2 times, most recently from 598ca60 to 22992c8 Compare May 2, 2024 14:25
@mcrumiller mcrumiller marked this pull request as ready for review May 2, 2024 14:44
Copy link

codecov bot commented May 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.33%. Comparing base (42795c6) to head (e717464).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16018      +/-   ##
==========================================
+ Coverage   80.75%   81.33%   +0.57%     
==========================================
  Files        1393     1403      +10     
  Lines      179459   183308    +3849     
  Branches     2924     2928       +4     
==========================================
+ Hits       144929   149089    +4160     
+ Misses      34027    33716     -311     
  Partials      503      503              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@alexander-beedie alexander-beedie left a comment

Choose a reason for hiding this comment

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

Could you tweak the unit tests to include a wider range of dates/datetimes (including pre-epoch, etc)? 👍

@mcrumiller
Copy link
Contributor Author

mcrumiller commented May 3, 2024

@alexander-beedie thanks, working on parametrizing and indeed running into cases where we're outside the Datetime casting ability, but I'm not sure how to deal with it. It looks like Date.cast(Datetime) fails (may open an issue) in general (@MarcoGorelli) (opened #16039):

from datetime import date
import polars as pl

pl.Series([date(1677, 9, 22)]).cast(pl.Datetime("ns")) # 1677-09-22 00:00:00
pl.Series([date(1677, 9, 21)]).cast(pl.Datetime("ns")) # 2262-04-11 23:34:33.709551616

@mcrumiller
Copy link
Contributor Author

@alexander-beedie fixed up a few things and expanded the tests. Let me know if you think it needs more testing or changes.

I restricted the times in the test to be within the Date -> Nanosecond allowance, as that can be addressed in the other issue I opened.

@alexander-beedie
Copy link
Collaborator

Sorry, let this one sit; can you rebase/update and I'll take another look :)

@mcrumiller
Copy link
Contributor Author

@alexander-beedie should be good once tests complete.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dtype ingored in pl.lit between date and datetime
2 participants