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

message.header.date should specify a timezone #143

Closed
jcb91 opened this issue Mar 21, 2016 · 11 comments
Closed

message.header.date should specify a timezone #143

jcb91 opened this issue Mar 21, 2016 · 11 comments
Milestone

Comments

@jcb91
Copy link

jcb91 commented Mar 21, 2016

This came up from ipython-contrib/jupyter_contrib_nbextensions#549, where the lack of timezone causes problems for any javascript trying to use the date value. I'm not super sure where the date field is added to the header, but assume it's either in jupyter_client/adapter.py#L387-L388 or in jupyter_client/jsonutil.py#L75-L80 - so apologies if this issue should actually be somewhere else.

Could the above calls be modified to use something like

from datetime import datetime
import pytz
pytz.utc.localize(datetime.now()).isoformat()

@Carreau
Copy link
Member

Carreau commented Mar 21, 2016

Hum, the alternative is to use datetime.utcnow() I guess.

But good point.

@jcb91
Copy link
Author

jcb91 commented Mar 21, 2016

Actually, datetime.utcnow() still doesn't return a timezone-aware datetime object (I've no idea why, really, UTC seems simple enough that it could be part of standard library, but hey ho). To remove dependency on pytz, something like this SO answer, which defines a tzinfo class for UTC, might be a better solution...

@jcb91
Copy link
Author

jcb91 commented Mar 21, 2016

see, for reference, the difference in the two print statements from this snippet:

from datetime import datetime, timedelta, tzinfo

TDZERO = timedelta(0)

class UTC(tzinfo):
    """A UTC tzinfo class"""

    def utcoffset(self, dt):
        return TDZERO

    def tzname(self, dt):
        return 'UTC'

    def dst(self, dt):
        return TDZERO

utc = UTC()

print('utcnow', datetime.utcnow().isoformat())
print('tz=utc', datetime.now(tz=utc).isoformat())

gives

utcnow 2016-03-21T19:03:04.625622
tz=utc 2016-03-21T19:03:04.625970+00:00

@Carreau
Copy link
Member

Carreau commented Mar 21, 2016

Actually, datetime.utcnow() still doesn't return a timezone-aware datetime object (I've no idea why, really, UTC seems simple enough that it could be part of standard library, but hey ho)

Sorry I was unclear in my statement, we could normalize the spec saying that the date time should be UTC if the current spec is not precise enough.
Of course for backward compatibility we should encourage people to have timezone, but I think in the long term, having UTC everywhere (execpt on user facing UI) make sens.

@jcb91
Copy link
Author

jcb91 commented Mar 21, 2016

Right, gotcha. Still, I think it's worth adding the timezone to the string values as well, rather than just altering the spec to allow people to assume they're in UTC - after all Explicit is better than implicit 😉

@takluyver
Copy link
Member

We already have three copies of the machinery to produce tz-aware UTC timestamps - there's one in jupyter_client, but it's currently in the tests (test_jsonutil to be precise). I'm pretty sure that UTC timestamps are the way to go, not localised timezone-aware timestamps.

@jcb91
Copy link
Author

jcb91 commented Mar 21, 2016

Oh absolutely, utc is the way forward, I just meant the offset (of zero, naturally) should get included in the json-ified text version, as with the utcnow in test_jsonutil.

@Carreau
Copy link
Member

Carreau commented Mar 22, 2016

Oh absolutely, utc is the way forward, I just meant the offset (of zero, naturally) should get included in the json-ified text version, as with the utcnow in test_jsonutil.

Do I smell a Pull Request coming ?

@jcb91
Copy link
Author

jcb91 commented Mar 22, 2016

Haha 😆 I'm happy to submit one, I was a little apprehensive of adding yet another mechanism to produce tz-aware timestamps! If you guys know where you'd like to put it, I'll be happy to fix it up...

@minrk
Copy link
Member

minrk commented Mar 22, 2016

We can have nice tz-aware utc timestamps with the stdlib in Python 3 with:

datetime.datetime.now(datetime.timezone.utc)

Python 2, of course, needs an backport of the trivial UTC timezone, which we have implemented here (and elsewhere). We can use utc for that, I suppose.

@minrk minrk added this to the 5.0 milestone Feb 10, 2017
@minrk
Copy link
Member

minrk commented Feb 10, 2017

master (will be 5.0) uses timezone-aware datetimes.

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

4 participants