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

Fixed #32366 -- Updated datetime module usage to recommended approach. #13916

Closed
wants to merge 1 commit into from

Conversation

carltongibson
Copy link
Member

@felixxm
Copy link
Member

felixxm commented Jan 19, 2021

Can we update all cases and tests ?

  • django/utils/http.py: current_year = datetime.datetime.utcnow().year
  • django/core/cache/backends/db.py: now = datetime.utcnow()
  • django/http/response.py: delta = expires - expires.utcnow()

@carltongibson
Copy link
Member Author

Yep, good call. I'll do that later on.

@ngnpope
Copy link
Member

ngnpope commented Jan 19, 2021

Can you also address datetime.utcfromtimestamp() and datetime.utctimetuple() which carry similar admonitions?

@carltongibson
Copy link
Member Author

Ok, super. I'm touching the methods using those now so...

I think I'll add a ticket.

@ngnpope
Copy link
Member

ngnpope commented Jan 19, 2021

Ok, super. I'm touching the methods using those now so...

🏅

I think I'll add a ticket.

Good call.

@carltongibson carltongibson changed the title Replaced .utcnow() call with .now(utc) Fixed #32366 -- Updated datetime module usage to recommended approach. Jan 19, 2021
@pganssle
Copy link
Contributor

Can you also address datetime.utcfromtimestamp() and datetime.utctimetuple() which carry similar admonitions?

I think utctimetuple() is a bit more complicated, but utcfromtimestamp() largely has all the same admonitions yes.

The biggest "legitimate" use case I have for utcnow() is when you are immediately attempting to use isoformat() on a datetime but don't want the offset included, e.g.: dt.utcnow().isoformat() + "Z", because .isoformat() will include +00:00 on an aware datetime in UTC. If we remove utcnow and utcfromtimetuple, then that will turn into dt.now(utc).replace(tzinfo=None).isoformat() + 'Z', which is slower and more awkward, but possibly worth it because these methods are mostly misused.

utcfromtimestamp() usually has one additional "legitimate" use case, which is that occasionally (and I occasionally use this in zoneinfo code), someone gives you a timestamp in "seconds since 1970-01-01T00:00:00 in the local zone". This is not a unix timestamp that would be generated by .timestamp, but IIRC the TZif format uses this to specify when transitions happen in "local time" for various reasons. It turns out that .utcfromtimestamp(that_kind_of_timestamp) will give you the correct datetime in local time, which is very convenient. Of course, utcfromtimestamp is a terrible name for a method that gives you the correct local time from something that is not really a "timestamp" in the sense of "timestamp" used by the .timestamp method...

Anyway, these are fairly rare use cases and any instance of utc(now|timestamp) can be replaced by utc(now|timestamp)(...).replace(tzinfo=None) if you actually want a naïve datetime representing UTC, if it's easier to enforce a ban on any use of .utc(now|timestamp), it will just be a bit slower.

@carltongibson
Copy link
Member Author

Thanks @pganssle. I ran out of brain power for this this afternoon but will look at each of the uses to see what we can update.

Base automatically changed from master to main March 9, 2021 06:21
@ngnpope
Copy link
Member

ngnpope commented May 10, 2021

Superseded by #14374.

@ngnpope ngnpope closed this May 10, 2021
@carltongibson carltongibson deleted the c/replace-utcnow branch May 11, 2021 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants