GH-103857: Deprecate utcnow and utcfromtimestamp

[pganssle]

Per the reasoning in #103857.

To do:

• Add tests
• Clean up internal uses of utcnow and utcfromtimestamp

• Issue: Deprecate utcnow and utcfromtimestamp #103857

[erlend-aasland]

LGTM (fixing the remaining test failures seems straight-forward.)

[ambv]

I like this, my nit is about the wordiness of the warnings.

Outside of that, there's the question of mixing tz-aware and tz-naive datetimes when it comes to comparisons and arithmetic operations. It might be somewhat disruptive for users to replace one with the other to discover TypeErrors in their code due to mixing naive and tz-aware datetimes.

[ambv]

I find this a little wordy for a warning, and tentative. The deprecated function will be removed in the future so we are not recommending an alternative, we are telling people to use an alternative. How about this:

Suggested change

	warnings.warn("datetime.utcfromtimestamp() is deprecated and scheduled "
	"for removal in a future version. It is instead "
	"recommended that users use timezone-aware objects to "
	"represent datetimes in UTC. To get such an object from "
	"a timestamp, use datetime.fromtimestamp(t, datetime.UTC).",
	DeprecationWarning,
	stacklevel=2)
	warnings.warn("datetime.utcfromtimestamp() is deprecated and will be "
	"removed in a future version. Use timezone-aware objects to "
	"represent datetimes in UTC: "
	"datetime.fromtimestamp(t, datetime.UTC)",
	DeprecationWarning,
	stacklevel=2)

[pganssle]

I did think it was a bit wordy when I wrote it. My only reservation here is that users still can use naïve objects as they want, they just need to do datetime.now(UTC).replace(tzinfo=None).

I suppose this subtle distinction is probably going to be lost on most people, so I'll just go with your wording.

[erlend-aasland]

I agree that Łukasz's suggestion is an improvement. I think it is ok to suggest the slightly disruptive change; we want people to use tz-aware objects¹.

Footnotes

1. Providing a drop-in replacement does not promote using tz-aware objects. ↩

[pganssle]

I agree that Łukasz's suggestion is an improvement. I think it is ok to suggest the slightly disruptive change; we want people to use tz-aware objects

I mean, in either case we're suggesting the change, the question is how best to communicate to users that the thing we're suggesting is not a drop-in replacement for the existing workflow. I tend to assume that when I see a deprecation warning with a code snippet in it, the code snippet is how you achieve the same thing in the new version. Ideally we'd make it clear that we are saying, "The thing you are doing now is not something you should continue doing, here's a new way to do it but you need to understand the implications of making this change."

That said, I'm not convinced that it's possible to convey this succinctly, so probably best to just leave it at Łukasz's wording.

[ambv]

Ditto.

Suggested change

	warnings.warn("datetime.utcnow() is deprecated and scheduled for "
	"removal in a future version. It is instead recommended "
	"that users use timezone-aware objects to represent "
	"datetimes in UTC. To get such an object representing "
	"the current time, use datetime.now(datetime.UTC).",
	DeprecationWarning,
	stacklevel=2)
	warnings.warn("datetime.utcnow() is deprecated and will be removed "
	"in a future version. Use timezone-aware objects to "
	"represent datetimes in UTC: datetime.now(datetime.UTC)",
	DeprecationWarning,
	stacklevel=2)

[ambv]

Ditto here about replacing the warning with the shorter version.

[ambv]

Ditto.

[pganssle]

Outside of that, there's the question of mixing tz-aware and tz-naive datetimes when it comes to comparisons and arithmetic operations. It might be somewhat disruptive for users to replace one with the other to discover TypeErrors in their code due to mixing naive and tz-aware datetimes.

Yes, this was somewhat hard to convey in the wording here. I am not trying to suggest that datetime.now(UTC) is a drop-in replacement for datetime.utcnow(). The drop-in replacement is datetime.now(UTC).replace(tzinfo=None). We're deprecating utcnow() because you should be using aware objects to represent datetimes that are actually aware, but it is a breaking change if you change your interfaces that return naïve datetimes into ones that return aware datetimes (which is why we're deprecating these instead of just making them return aware datetimes 😅). This is one reason I worded the warning the way I did.

Another option is to just say, "Use timezone-aware datetimes instead" and not include the code for how to do that, so that they are forced to look up how to do that and are less likely to think it's a drop-in replacement? I suspect that doing so would be a somewhat frustrating user expreience, though. Tough call.

[pganssle]

@ambv (or others), what do you think of this wording?

datetime.utcnow() is deprecated and scheduled for removal in a future version.
Instead, Use timezone-aware objects to represent datetimes in UTC:
datetime.now(datetime.UTC). This is not a drop-in replacement.

Alternatively, This will not be a backwards-compatible change. or something to that effect?

[pganssle]

OK, I'm going to merge this when CI passes, using Łukasz's wording, unless there are some objections.

[ankit-gautam23]

LGTM