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

depr(rust, python): warn that, in a future version of Polars, constructing a Series with time-zone-aware datetimes will result in a dtype with UTC timezone #8908

Merged

Conversation

MarcoGorelli
Copy link
Collaborator

towards #8860

This will always raise a warning, which may be a bit annoying, but it's a custom warning class so users can filter it off if they wish (this is arguably better than making a breaking change with no warning?)

…cting a Series with time-zone-aware datetimes will result in a dtype with UTC timezone
@MarcoGorelli MarcoGorelli mentioned this pull request May 18, 2023
7 tasks
@ritchie46
Copy link
Member

Right, and on 0.18 we will not have this warning anymore right?

@MarcoGorelli
Copy link
Collaborator Author

It depends when you want to enforce the change - warning in 0.17.15, then enforcing in 0.18?
Would you want to do the same for the other points in #8860?

TBH I'd be OK with that: so 0.18.x would be there to iron-out any issues, and 1.0.0 would be a solid release

],
"b": [float(i) for i in range(5)],
}
)
.with_columns(pl.col("a").dt.replace_time_zone(time_zone))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

setting the time zone outside the constructor to avoid the warning (the constructor isn't the point of this test anyway) - likewise for the one below

@ritchie46
Copy link
Member

It depends when you want to enforce the change - warning in 0.17.15, then enforcing in 0.18? Would you want to do the same for the other points in #8860?

TBH I'd be OK with that: so 0.18.x would be there to iron-out any issues, and 1.0.0 would be a solid release

I think that'd be fine. That would allow us to iron out these wrinkles.

Now that I think of this, we should keep this one as a warning I think. User should be able to pass tz aware datetimes, but they must be aware that we convert them, so a warning does that.

@MarcoGorelli
Copy link
Collaborator Author

Now that I think of this, we should keep this one as a warning I think

Sounds good!

@MarcoGorelli
Copy link
Collaborator Author

thanks for reviewing - shipping then!

@MarcoGorelli MarcoGorelli merged commit 81a2450 into pola-rs:main May 19, 2023
9 checks passed
ritchie46 pushed a commit that referenced this pull request May 20, 2023
…cting a Series with time-zone-aware datetimes will result in a dtype with UTC timezone (#8908)
alexander-beedie pushed a commit to alexander-beedie/polars that referenced this pull request May 20, 2023
…cting a Series with time-zone-aware datetimes will result in a dtype with UTC timezone (pola-rs#8908)
@stinodego stinodego added the python Related to Python Polars label Jul 14, 2023
c-peters pushed a commit to c-peters/polars that referenced this pull request Jul 14, 2023
…cting a Series with time-zone-aware datetimes will result in a dtype with UTC timezone (pola-rs#8908)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants