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

Interpret naive datetimes as local time #257

Open
adamchainz opened this issue May 31, 2022 · 4 comments
Open

Interpret naive datetimes as local time #257

adamchainz opened this issue May 31, 2022 · 4 comments

Comments

@adamchainz
Copy link
Owner

Description

Python defaults to interpreting naive datetimes as local time: https://blog.ganssle.io/articles/2022/04/naive-local-datetimes.html

time-machine currently turns naive datetimes into UTC, except from naive datetiems parsed from strings with datetutil, which it keeps in local times: https://twitter.com/hugovk/status/1513513262649950210

@pganssle would prefer time-machine to be consistent with Python's behaviour: https://twitter.com/pganssle/status/1513507255253184518

I'm inclined to agree. It would be good to at least be consistent between datetimes and strings. It would be a breaking change though.

We might also consider a warning for naive datetimes, as this can allow a test's success to depend on the timezone in which it is run.

@PhML
Copy link

PhML commented Jul 13, 2022

I am not sure it is related, but this is very confusing:

In [1]: from datetime import datetime

In [2]: from freezegun import freeze_time

In [3]: import time_machine

In [4]: fake_now = datetime(2022, 7, 12)

In [5]: @freeze_time(fake_now)
   ...: def test_freeze_time():
   ...:     return datetime.now().isoformat()
   ...:

In [6]: @time_machine.travel(fake_now)
   ...: def test_time_machine():
   ...:     return datetime.now().isoformat()
   ...:

In [7]: fake_now.isoformat()
Out[7]: '2022-07-12T00:00:00'

In [8]: test_freeze_time()
Out[8]: '2022-07-12T00:00:00'

In [9]: test_time_machine()
Out[9]: '2022-07-12T02:00:00'

The time_machine traveled datetime.now is 2 hours in advance from my fake_now datetime.

@soxofaan
Copy link
Contributor

soxofaan commented Dec 1, 2022

Same head-scratching over here (local timezone: UTC+1):

for destination in [
    dt.datetime(2000, 1, 1),
    "2000-01-01",
    "2000-01-01 00+00",
]:
    print(f"Travel to {destination!r}")
    with time_machine.travel(destination, tick=False):
        print(f"  utcnow:   {dt.datetime.utcnow()}")
        print(f"  now:      {dt.datetime.now()}")

output = """
Travel to datetime.datetime(2000, 1, 1, 0, 0)
  utcnow:   2000-01-01 00:00:00
  now:      2000-01-01 01:00:00
Travel to '2000-01-01'
  utcnow:   1999-12-31 23:00:00
  now:      2000-01-01 00:00:00
Travel to '2000-01-01 00+00'
  utcnow:   2000-01-01 00:00:00
  now:      2000-01-01 01:00:00
"""

It's annoying that the quick and easy way of specifying a date as string (second case with "2000-01-01" in example) does not work as expected, and one should at least explicitly have a timezone in the string (00+00) or use datetime object.

I think there are multiple (related) issues raised here:

  1. The current docs wrongly describe current behavior:

    destination specifies the datetime to move to. It may be: ...

    • A string, which will be parsed with dateutil.parse and converted to a timestamp. Again, if the result is naive, it will be assumed to have the UTC time zone.

    the example above (and the tweets mentioned higher) show that naive is assumed to be local time, not UTC.

  2. Inconsistent behavior between handling a naive destination: a naive datetime object is assumed to be UTC, a naive string is assumed to be local time. This is counter-intuitive to say at least.

  3. If I understand correctly, the OP (title "... naive ... as local time") is about changing the handling of all naive destination as local instead of UTC.

Fixing 1. (without tackling 2. and 3. yet) is easy I guess. But fixing 2. or 3. would indeed breaking changes.

Personally, I'd prefer to handle all naive destinations as UTC, because that makes most sense in a test/CI context. But I understand the desire to be consistent with Python conventions.

@soxofaan
Copy link
Contributor

soxofaan commented Dec 1, 2022

If you would go for fixing 3. from my previous comment (so travel(destination=...) would interpret naive as local in all cases), could it be an option to introduce a variant travel_utc(destination=...) (or a classmethod factory travel.utc(destination=...)) that would interpret all naive input as UTC instead?

@adamchainz
Copy link
Owner Author

could it be an option to introduce a variant travel_utc(destination=...) (or a classmethod factory travel.utc(destination=...)) that would interpret all naive input as UTC instead?

I wouldn’t like to complicate things, no. You could use a wrapper in your projcets.

soxofaan added a commit to soxofaan/time-machine that referenced this issue Dec 2, 2022
soxofaan added a commit to soxofaan/time-machine that referenced this issue Dec 2, 2022
adamchainz pushed a commit to soxofaan/time-machine that referenced this issue Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants