Skip to content

Naive datetime should be treated as local timezone. UTC datetimes should be timezone aware. #1658

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.

Sign up for GitHub

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

Jump to bottom
Closed
n5y opened this issue Dec 8, 2019 · 6 comments
Closed

Naive datetime should be treated as local timezone. UTC datetimes should be timezone aware. #1658

n5y opened this issue Dec 8, 2019 · 6 comments
Milestone
13.0

Comments

@n5y
Copy link
Contributor

n5y commented Dec 8, 2019 •
edited
Loading

Caused by #1506(#1505) #1485(#1362)

I believe local timezone is the expected default for naive datetime objects. Breaking those expectations will lead to unnecessary surprises for the users. It will also lead to awkward conversions if another library that does follow this convention is used.

Python's datetime.fromtimestamp and datetime.astimezone assume local timezone for naive objects. There are three warnings in datetime docs that start with "Because naive datetime objects are treated by many datetime methods as local times, it is preferred to use aware datetimes to represent times in UTC."

Using utc time is surely more sane than using local time, but the better way to do that is timezone aware datetimes, not naive-utc datetimes. For example Message.date should be timezone aware.
@Bibo-Joshi Bibo-Joshi added this to the 13.0 milestone Feb 20, 2020
@Bibo-Joshi
Copy link
Member

As of #1621 (released in v12.7), helpers.from_timestamp returns an aware object. Therefore I see this issue as resolved and will close.
If I'm missing the point and/or you disagree, feel free to comment.

@n5y
Copy link
Contributor Author

n5y commented Jul 15, 2020

I disagree, that pull request only addressees the second sentence from the issue title. Using aware datetimes removes the need to use "naive-utc", but at the moment naive datetimes are still treated as utc by the library.

On a side note: update.message.date changing from naive to aware is a breaking change and is not really explained in release notes.

@Bibo-Joshi
Copy link
Member

Sorry, I don't really understand what you mean by "but at the moment naive datetimes are still treated as utc by the library." Or are you talking about JobQueue?

I'll update the changelog.

@n5y
Copy link
Contributor Author

n5y commented Jul 15, 2020 •
edited
Loading

I'm talking about datetime objects being provided by a user. I guess it's only relevant to job queue stuff at the moment. It also affects any future methods that will accept a datetime/time from a user, since those methods will probably use the same helpers.

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Jul 16, 2020 •
edited
Loading

Mh okay. E.g. send_polls close_date and kick_chat_members until_date would also fall into that category. Didn't have that in mind. Fair enough, I'll reopen.

Just FYI: We're planning on adding tzinfo to the Defaults logic, which would allow to interpret all naive date/time objects in the library as a customized timezone. I know that's not exactly what you proposed, but it would make it easy to have everything interpreted as local timezone. (See #1797 )

@Bibo-Joshi Bibo-Joshi reopened this Jul 16, 2020
@Bibo-Joshi
Copy link
Member

We discussed this again internally and decided to stick with the way it is now, i.e. tread naive datetimes as UTC by default.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 17, 2020
Bibo-Joshi referenced this issue Dec 6, 2020
@Bibo-Joshi
Defaults.tzinfo (#2042)
103b115
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
13.0
Development

No branches or pull requests
2 participants
@Bibo-Joshi @n5y