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

Inconsistent use of UTC/local naïve datetimes #1505

Closed
plammens opened this issue Sep 4, 2019 · 2 comments · Fixed by #1506
Closed

Inconsistent use of UTC/local naïve datetimes #1505

plammens opened this issue Sep 4, 2019 · 2 comments · Fixed by #1506

Comments

@plammens
Copy link
Contributor

plammens commented Sep 4, 2019

PR #1485 made telegram.utils.helpers.from_timestamp return a naïve datetime.datetime object in UTC, but this created an inconsistency with the rest of the repository in the handling of naïve datetime.datetimes, where it is assumed that these objects represent local time. In fact, there is a direct inconsistency between from_timestamp and to_timestamp helpers, since the latter still assumes that the datetime object is in local time.

See #1497 (comment) (point 3.) for extended discussion.

I've already fixed the issue in #1497 (and added tests), but should I make a separate PR to isolate the change?

Steps to reproduce

  1. Go to a place with nonzero UTC offset. I live in CEST (GMT +2).

  2. Construct a telegram.Message from a JSON data dictionary. This is a message I sent to a test bot of mine (stripped of the optional fields), as received from an HTTP request to Telegram:

    import telegram as tg
    
    message_json = {
        'message_id': 462,
        'from': {
            'id': 41214001,
            'is_bot': False,
            'first_name': 'Paolo',
        },
        'chat': {
            'id': 41214001,
            'type': 'private'
        },
        'date': 1567596574,
    }
    
    original_timestamp = message_json['date']
    
    bot = tg.Bot('000:not_a_token')
    message = tg.Message.de_json(message_json, bot=bot)
  3. Re-form the Message's JSON dict and extract the timestamp:

    restored_timestamp = message.to_dict()['date']
  4. Check that the timestamps are the same:

    assert restored_timestamp == original_timestamp, \
        f"Timestamps differ by {restored_timestamp - original_timestamp} seconds"

Expected behaviour

The timestamps should be the same.

Actual behaviour

The timestamps differ by the local UTC offset (reversed):

AssertionError: Timestamps differ by -7200 seconds

When the Message is constructed, the datetime object it gets represents UTC time, but when it gets re-converted to a timestamp, that same object is assumed to be in local time, which in my case is 7200 seconds ahead of UTC, so the resulting timestamp is 7200 seconds behind the original.

Configuration

Operating System: Windows

Version of Python, python-telegram-bot & dependencies:

$ python -m telegram

python-telegram-bot 12.0.0 (v12.0.0-0-gc84e21d)
certifi 2019.03.09
future 0.17.1
Python 3.7.3 (v3.7.3:ef4ec6ed12, Mar 25 2019, 22:22:05) [MSC v.1916 64 bit (AMD64)]
@Bibo-Joshi
Copy link
Member

Yes, I think putting the time-handling overhaul in a separate PR would be good.

@plammens
Copy link
Contributor Author

plammens commented Sep 4, 2019

Yes, I think putting the time-handling overhaul in a separate PR would be good.

Ok. By #1409 (comment), I understand we want the standard for naïve datetimes to be UTC, not local, right? (While also handling aware datetimes correctly)

plammens added a commit to plammens/python-telegram-bot that referenced this issue Sep 4, 2019
(cherry-picked from c6dd3d7.
I've included the refactoring mentioned in python-telegram-bot#1497 to facilitate the
change.)

There was inconsistent use of UTC vs local times. For instance, in the
former `_timestamp` helper (now `_datetime_to_float_timestamp`), assumed
that naive `datetime.datetime` objects were in the local timezone, while
the `from_timestamp` helper —which I would have thought was the
corresponding inverse function— returned naïve objects in UTC.

This meant that, for instance, `telegram.Message` objects' `date` field
was constructed as a naïve `datetime.datetime` (from the timestamp sent
by Telegram's server) in *UTC*, but when it was stored in `JSON` format
through the `to_json` method, the naïve `date` would be assumed to be in
*local time*, thus generating a different timestamp from the one it was
built from.

See python-telegram-bot#1505 for extended discussion.
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants