python / cpython

The Python programming language
https://www.python.org
Other
62.69k stars 30.07k forks source link

`utcnow` deprecation note is misleading #118542

Open srittau opened 5 months ago

srittau commented 5 months ago

Documentation

Currently, the deprecation notice for datetime.utcnow() reads:

Deprecated since version 3.12: Use datetime.now() with UTC instead.

Unfortunately, applying this suggestion verbatim can easily break user code – especially when applying it to libraries. (See for example https://foss.heptapod.net/openpyxl/openpyxl/-/issues/2051). These datetime-related TypeErrors are easy to introduce, and fairly hard to fix. I suggest to amend the deprecation notice:

Deprecated since version 3.12: Use datetime.now(UTC) instead. Please note that this will return an aware datetime object. If you need to remain compatible, use datetime.now(UTC).replace(tzinfo=None).

Maybe someone can come up with a better wording.

Linked PRs

pganssle commented 5 months ago

It was definitely a deliberate decision to avoid suggesting a drop-in replacement. I had thought that the wording also made it clear that making this change would be a brain change to your interfaces, though.

The reason to avoid suggesting a drop-in replacement is because the main issue with utcnow isn't so much the function itself but rather the actual thing that it does. People need to move away from using native datetimes to represent UTC entirely, not just the utcnow function.

srittau commented 5 months ago

It's not that I disagree, but the current wording doesn't make the impact of the change clear. Changing a naïve datetime to a tz aware one is a major change and very likely to break user code if not done carefully. The situation is actually quite comparable to the Python 2 unicode/str transition, but of course on a much smaller scale.

Also, at the moment, naïve datetimes are not deprecated as far as I'm aware. And while that may be a worthwhile goal, I think this would require a PEP, considering the impact of that change. And as long as it's not deprecated I don't think we should force users into a transition they might not be interested in.

pganssle commented 4 months ago

It's not that I disagree, but the current wording doesn't make the impact of the change clear. Changing a naïve datetime to a tz aware one is a major change and very likely to break user code if not done carefully. The situation is actually quite comparable to the Python 2 unicode/str transition, but of course on a much smaller scale.

I am not saying that the message can't be improved, just that we should not be offering a drop-in replacement method. Users need to think about how to make the change, and if we offer a "no thought required" option they will take it.

Also, at the moment, naïve datetimes are not deprecated as far as I'm aware. And while that may be a worthwhile goal,

Correct, they are not deprecated, and indeed it would not be a worthwhile goal. My suggestion is not to eliminate naïve datetimes, but to stop people from using them wrong. If there were a method in the standard library that attached America/New_York to a datetime, but returned a datetime that you were supposed to treat like it is UTC, deprecating that method and suggesting that you use the correct abstractions would not be confused for "deprecating aware datetimes" or "deprecating the use of America/New_York". The situation here is exactly analogous.

srittau commented 4 months ago

I am not saying that the message can't be improved, just that we should not be offering a drop-in replacement method. Users need to think about how to make the change, and if we offer a "no thought required" option they will take it.

Very often the change will be "I still want a naïve datetime object", either to guarantee a stable API, or to easily upgrade to Python 3.12, without the additional hassle of updating the usage of datetime objects throughout the code. Especially since naïve datetime objects are not deprecated and will continue to work for the foreseeable future. Having users to jump through additional hoops to fulfill their goal, and offering only the dangerous solution in the documentation, instead of enabling users to make an informed decision themselves sounds quite patronising, to be honest. If users will take the "easy solution" that's their prerogative.

vadmium commented 4 months ago

Have you considered just dropping the descriptions of utcnow and utcfromtimestamp, and replacing them with something like

datetime.utcnow() Equivalent to datetime.now(UTC).replace(tzinfo=None).

Deprecated since 3.12.

datetime.utcfromtimestamp(timestamp) Equivalent to datetime.fromtimestamp(timestamp, UTC).replace(tzinfo=None).

Deprecated since 3.12.

Perhaps also rescue the description of fromtimestamp(. . ., timezone.utc) which doesn’t really belong under utcfromtimestamp, and remove the opening claim suggesting fromtimestamp’s date and time is always local.