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

Deprecate utcnow and utcfromtimestamp #103857

Open
pganssle opened this issue Apr 25, 2023 · 2 comments
Open

Deprecate utcnow and utcfromtimestamp #103857

pganssle opened this issue Apr 25, 2023 · 2 comments
Labels
type-feature A feature request or enhancement

Comments

@pganssle
Copy link
Member

pganssle commented Apr 25, 2023

Feature or enhancement

Previously, we have documented that utcnow and utcfromtimestamp should not be used, but we didn't go so far as to actually deprecate them, and I wrote a whole article about how you shouldn't use them.

The main reason I had for not deprecating them at the time was that .utcnow() is faster than .now(datetime.UTC), and if you are immediately converting the datetime to a string, like datetime.utcnow().isoformat(), there's no danger.

I have come around to the idea that this type of use case is not important enough to leave the attractive nuisances of utcnow() and utcfromtimestamp() in place, and we should go ahead and deprecate them.

Pitch

We should deprecate them in the documentation and also add DeprecationWarnings imploring people not to use them. I'm OK with us saying that we will remove them "at some point in the future" and not necessarily putting a deadline on it, considering how much use of utcnow() is out there.

Previous discussion

Linked PRs

@pganssle pganssle added the type-feature A feature request or enhancement label Apr 25, 2023
@pganssle
Copy link
Member Author

I've gone ahead and done some benchmarks for the most common legitimate use case I've seen for utcnow and utcfromtimestamp, which is getting a naïve datetime and immediately formatting it to a string with no time zone offset. The alternatives are considerably slower, though I don't know how important that is for real life use cases:

>>> %timeit datetime.now(UTC).replace(tzinfo=None).isoformat(' ')
2.15 µs ± 19.9 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
>>> %timeit datetime.now(UTC).isoformat(' ')[:-6]
1.61 µs ± 23.7 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
>>> %timeit datetime.utcnow().isoformat(' ')
919 ns ± 5.23 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

As an example of how this changes the speed in a real-life application, here are the before and after measurements for the change to http.cookiejar.time2isoz:

>>> t = datetime.now().timestamp()
>>> %timeit cookiejar(None)  # Uses datetime.now
1.52 µs ± 16.2 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
>>> %timeit cookiejar_utc(None)  # Uses datetime.utcnow
1.32 µs ± 6.72 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

>>> %timeit cookiejar(t)  # Uses datetime.fromtimestamp
1.77 µs ± 24.2 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
>>> %timeit cookiejar_utc(t)  # Uses datetime.utcfromtimestamp
1.4 µs ± 5.75 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

I'm still more or less convinced that this is useful to do, and I'd like to see if anyone complains that it's a major problem after the deprecation before worrying about these micro-optimizations.

pganssle added a commit that referenced this issue Apr 27, 2023
Using `datetime.datetime.utcnow()` and `datetime.datetime.utcfromtimestamp()` will now raise a `DeprecationWarning`.

We also have removed our internal uses of these functions and documented the change.
@pganssle
Copy link
Member Author

#103858 is now merged, but we still need a What's New entry before closing this, I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant