bpo-37488 : Document a warning for datetime.utcnow() and utcfromtimestamp() by nanjekyejoannah · Pull Request #15773 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation16 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
@nanjekyejoannah I don't think we can (or possibly should) put an actual warning in the code here - people will likely complain about it and we're not actually deprecating the function yet, it will likely just cause a lot of uproar and spam (as much as I'd like to...).
What I was referring to in bpo-15733 was a warning box in the documentation, like:
.. warning::
Because naive datetimes are treated by many functions as local times,
it is recommended that you create a timezone-aware datetime using
``datetime.now(tz=timezone.utc)`` instead.I have not spent much time thinking about this. We may want a little footnote showing one of the gotchas to be referenced by both utcfromtimestamp and utcnow.
| @@ -29,8 +29,9 @@ is used to represent a specific moment in time that is not open to |
|---|
| interpretation [#]_. |
| A naive object does not contain enough information to unambiguously locate |
| itself relative to other date/time objects. Whether a naive object represents |
| Coordinated Universal Time (UTC), local time, or time in some other timezone is |
| itself relative to other date/time objects. Native objects are no longer |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this an intentional change? It looks like you kept in the beginnings of an explanation of the problem, but also the problematic old text.
This is actually a bit complicated to reword, because the sentiment behind it has changed, and we may need to hedge a bit. Let me think about how best to communicate the new intent....
Okay. I had thought you wanted us to introduce a warning. I will update this PR accordingly.
bpo-37488 : Add warning to datetime.utcnow() and datetime.utcfromtimestamp() bpo-37488 : Document a warning for datetime.utcnow()
bpo-37488 : Document a warning for datetime.utcnow() bpo-37488 : Document a warning for datetime.utcnow() and utcfromtimestamp()
| @@ -804,6 +811,11 @@ Other constructors, all class methods: |
|---|
| except the latter formula always supports the full years range: between |
| :const:`MINYEAR` and :const:`MAXYEAR` inclusive. |
| .. warning:: |
| Because naive datetimes are treated by many functions as local times. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this wording to be of a similar sentence structure as in the earlier section (for datetime.utcnow())?
Documentation in the Doc dir
label
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -752,6 +752,13 @@ Other constructors, all class methods: |
|---|
| :class:`.datetime` object. An aware current UTC datetime can be obtained by |
| calling ``datetime.now(timezone.utc)``. See also :meth:`now`. |
| .. warning:: |
| Because naive datetimes are treated by many functions as local times, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a bit more specific. Also, code comments or Sphinx roles should be used when referencing objects (such as classes, methods, or functions). In this case, code comments would be more appropriate since this is already within the section for datetime.
| Because naive datetimes are treated by many functions as local times, |
|---|
| Because naive ``datetime`` objects are treated by many ``datetime`` methods as local times, |
This line will need to be split off into the next one since it will exceed the reST maximum width of 80 chars (94 chars after the suggestion).
@nanjekyejoannah I believe this will have merge conflicts now that #13410 has been merged. Can you rebase against master? I also think adding a warning in utctimetuple() will allow us to close #10870.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nanjekyejoannah For consistency, can provide the warning message for each of them?
Because naive
datetimeobjects are treated by manydatetimemethods
as local times, the recommended way to create aware datetime objects for
the current time in UTC is by callingdatetime.now(timezone.utc).
Otherwise, everything else looks good to me.
DinoV pushed a commit to DinoV/cpython that referenced this pull request
Thanks @nanjekyejoannah for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
Labels
Documentation in the Doc dir