GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — GH-103857: Deprecate utcnow and utcfromtimestamp by pganssle · Pull Request #103858 · python/cpython Conversation
|
| 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) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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:
| 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) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
| 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) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| PyErr_WarnEx( | ||
| PyExc_DeprecationWarning, | ||
| "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).", | ||
| 2 | ||
| ); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ditto here about replacing the warning with the shorter version.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| PyErr_WarnEx( | ||
| PyExc_DeprecationWarning, | ||
| "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).", | ||
| 2 | ||
| ); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yes, this was somewhat hard to convey in the wording here. I am not trying to suggest that 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@ambv (or others), what do you think of this wording?
Alternatively, This will not be a backwards-compatible change. or something to that effect? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
66a82a0 to
3c7c703
Compare
688bb05 to
b223173
Compare
|
OK, I'm going to merge this when CI passes, using Łukasz's wording, unless there are some objections. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.