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

utcnow deprecation note is misleading #118542

Open
srittau opened this issue May 3, 2024 · 5 comments · May be fixed by #118571
Open

utcnow deprecation note is misleading #118542

srittau opened this issue May 3, 2024 · 5 comments · May be fixed by #118571
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes docs Documentation in the Doc dir

Comments

@srittau
Copy link
Contributor

srittau commented May 3, 2024

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

@srittau srittau added the docs Documentation in the Doc dir label May 3, 2024
@AlexWaygood AlexWaygood added 3.12 bugs and security fixes 3.13 bugs and security fixes labels May 3, 2024
@pganssle
Copy link
Member

pganssle commented May 6, 2024

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
Copy link
Contributor Author

srittau commented May 6, 2024

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
Copy link
Member

pganssle commented May 6, 2024

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
Copy link
Contributor Author

srittau commented May 6, 2024

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
Copy link
Member

vadmium commented May 14, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes docs Documentation in the Doc dir
Projects
Development

Successfully merging a pull request may close this issue.

4 participants