gh-118542: improve datetime deprecation warnings #118571
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>
Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>
|
Could you also apply a similar change to the deprecation notice in the datetime module documentation? |
There was a problem hiding this comment.
I do think we can make it clearer that switching to timezone-aware datetimes is a breaking change if the datetimes you are using them with are exposed to end users, I am still firmly against the idea of anything that looks like recommending an alternative way to get naive datetimes that represent UTC.
| @@ -1809,7 +1809,9 @@ def utcfromtimestamp(cls, t): | |||
| warnings.warn("datetime.datetime.utcfromtimestamp() is deprecated and scheduled " | |||
| "for removal in a future version. Use timezone-aware " | |||
| "objects to represent datetimes in UTC: " | |||
| "datetime.datetime.fromtimestamp(t, datetime.UTC).", | |||
| "datetime.datetime.fromtimestamp(t, datetime.UTC). To maintain " | |||
| "compatibility datetime.datetime.fromtimestamp(timestamp) " | |||
There was a problem hiding this comment.
It was observed, if no timezone is provided, fromtimestamp returns a naïve datetime object.
| @@ -985,7 +986,8 @@ Other constructors, all class methods: | |||
|
|
|||
| .. deprecated:: 3.12 | |||
|
|
|||
| Use :meth:`datetime.fromtimestamp` with :attr:`UTC` instead. | |||
| Use :meth:`datetime.fromtimestamp` with :attr:`UTC` instead. To maintain | |||
| compatibility :meth:`datetime.fromtimestamp`without :attr:`UTC` may be used. | |||
There was a problem hiding this comment.
If we do decide to offer this approach, I would be much clearer in the message that doing this is effectively equivalent to suppressing the warning without fixing the underlying issue.
There was a problem hiding this comment.
That's true, this issue was raised (and I had the same experience) because there was no in-place replacement to maintain compatibility provided. I ended up at the same conclusion currently proposed several weeks before the original issue was opened but after some significant time.
Is this the correct understanding here; that the proposed modifications correctly address the case where code written for py<3.12 being run on py>=3.12. But do not adequately address (or propose a misleading solution when) the deprecated functions being called when new code is being written in py>=3.12?
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
| @@ -5199,7 +5199,8 @@ datetime_utcnow(PyObject *cls, PyObject *dummy) | |||
| if (PyErr_WarnEx(PyExc_DeprecationWarning, | |||
| "datetime.datetime.utcnow() is deprecated and scheduled for removal in a " | |||
| "future version. Use timezone-aware objects to represent datetimes " | |||
| "in UTC: datetime.datetime.now(datetime.UTC).", 1)) | |||
| "in UTC: datetime.datetime.now(datetime.UTC). To maintain compatibility" | |||
| "datetime.datetime.now(datetime.UTC).replace(tzinfo=None) may be used.", 1)) | |||
There was a problem hiding this comment.
It looks like this will have two words joinedtogether.
But do the warning messages really need to specify replacement options at all?
Add suggestion for code authors to have an in-place, future-proof replacement if desired.
Builds on the initial issue raised by also adding compatibility suggestion for
datetime.datetime.fromtimestamp(timestamp, datetime.UTC)->datetime.datetime.fromtimestamp(timestamp). By not providing a timezone, the returned object is not timezone aware.fixes #118542
utcnowdeprecation note is misleading #118542