Skip to content

gh-124832: Add a note to indicate that datetime.now may return the same instant #124834

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

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

spacemanspiff2007
Copy link
Contributor

@spacemanspiff2007 spacemanspiff2007 commented Oct 1, 2024

Made the implied behavior more clear


📚 Documentation preview 📚: https://cpython-previews--124834.org.readthedocs.build/

replace warning with note
Co-authored-by: Victor Stinner <[email protected]>
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@spacemanspiff2007
Copy link
Contributor Author

I'm not a native speaker, but maybe it sounds better if we turn the sentence around?
However the important part is then at the end of the sentence which is not optimal, too.
I also added "statements". What do you think?

Depending on the precision of the underlying clock, it's possible that subsequent datetime.now() statements return the same instant.

Comment on lines 939 to 940
It's possible that subsequent ``datetime.now()`` return the same instant,
depending on the precision of the underlying clock.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:
Depending on the precision of the underlying clock, it's possible that subsequent datetime.now() statements return the same instant.

@pganssle
Copy link
Member

pganssle commented Oct 3, 2024

Looking at the docs, this doesn't seem to be cluttering things up too much, but I do wonder what the threshold should be for this sort of thing. It seems like it would be obvious that if you create two datetime objects in a period of time shorter than the resolution of datetime, they will have the same value.

It also seems like you could get datetime objects out of order if the underlying system clock changes.

To me, it feels like the kinds of edge cases we should be warning against are:

  1. Edge cases where there is an inherent ambiguity, and we want to document the degree to which we've made a choice about how to resolve it (e.g. "what does .timestamp() do on a naïve datetime?", "what is the range of supported dates"?)
  2. Stuff that is such an extremely common mistake that is essentially a problem with our API that people don't realize they are getting the wrong thing.

I am not convinced that this warning meets either threshold, to be honest.

Maybe the @python/editorial-board wants to weigh in on the level of granularity we want with these docs, but I feel like if we start listing out all the edge cases where someone could make a mistake, the documentation will eventually be nothing but that.

@gvanrossum
Copy link
Member

I personally agree with Paul.

@vstinner
Copy link
Member

vstinner commented Oct 3, 2024

On Windows in Python 3.13, I modified the clock used by datetime to have a resolution around 238 ns (under 1 microsecond) instead of 15.6 ms:
https://docs.python.org/dev/whatsnew/3.13.html#time

It makes this issue way less likely.

@willingc
Copy link
Contributor

willingc commented Oct 3, 2024

Thanks for the PR @spacemanspiff2007. As a member of the docs editorial board, I'm a bit hesitant to add this note to the docs. As a developer, I expect that .now() will return the current time at the moment of execution based on the clock's precision. While in some cases of low precision clocks, the return value could be the same if called rapidly in succession. My biggest concern with the note is that it will open up more questions than clarifications.

@spacemanspiff2007
Copy link
Contributor Author

It seems like it would be obvious that if you create two datetime objects in a period of time shorter than the resolution of datetime, they will have the same value.

Well - it was not obvious to me and also not obvious to the other people who maintain the library where I created an issue because of this surprising behavior which I suspected to be a bug.
If I look at the datetime.now() docs there is some talk about precision and that it might increase.
Which should be removed, too, because it's rather confusing and also doesn't add any insight or clarity.
Also it's not clear that under Python 3.12 the precision is 15.6ms which is actually a lot and imho more than justifies a note.

While in some cases of low precision clocks, the return value could be the same if called rapidly in succession.

Can you show me the location in the docs where it shows that datetime.now() behaves this way?
You know that it behaves like that, but how do I as a non python expert gain this knowledge?

My biggest concern with the note is that it will open up more questions than clarifications.

Maybe the goal should be to improve the description then?

@picnixz picnixz changed the title gh-124832: Added warning that .now() can return the same instant gh-124832: Add a note to indicate that datetime.now may return the same instant Oct 3, 2024
@picnixz
Copy link
Member

picnixz commented Oct 3, 2024

(I've edited the title since it's neither a runtime nor a docs warning).

@gvanrossum
Copy link
Member

Still speaking for myself, while I agree that the proposed new text is clearer, I still don’t understand why this warning is needed. The phenomenon is rather obvious, unless you assume the clock has infinite precision.

I could get behind a more general discussion of clock precision and its implications though, especially noting the 3.12 and before coarse precision on Windows. (That even deserves a “changed in 3.13 note”).

Co-authored-by: Bénédikt Tran <[email protected]>
@spacemanspiff2007
Copy link
Contributor Author

The phenomenon is rather obvious, unless you assume the clock has infinite precision.

datetime provides microsecond and it does not state in the datetime docs that the precision might be less than that.
The only logical conclusion is that datetime.now() will have microsecond precision.

If you look at time.time() it states that precision might be as low as 1s even though it's a float. There the docs make it clear.
How about we add the same two sentences to datetime instead of a note box? I've made a suggestion.

@gvanrossum
Copy link
Member

Looks like a confusion between accuracy and precision. Possibly the docs weren’t written with that distinction clear. For which our apologies, we should do better about that.

still, how many times do you think you can call that function on a microsecond? Try to measure that (using a larger time interval ;-) on Linux.

@spacemanspiff2007
Copy link
Contributor Author

still, how many times do you think you can call that function on a microsecond? Try to measure that (using a larger time interval ;-) on Linux.

What I'm saying is that the datetime type implies a nanosecond precision/accuracy and the docs never state otherwise.
I know that I can call datetime.now() more than once in 15.6ms because the unexpected behavior is what resulted in me creating the issue.

As for what it's worth noting in the docs:
I was not aware of the restriction nor was the person maintaining another library.
If you guys think that's common knowledge then obviously we have some catching up to do - it would be nice if you could point me to a source where that behavior is properly described.

In any case please feel free to close this PR.

@willingc
Copy link
Contributor

willingc commented Oct 4, 2024

Looks like a confusion between accuracy and precision. Possibly the docs weren’t written with that distinction clear. For which our apologies, we should do better about that.

It probably makes the most sense to rewrite the method description (starting at line 926). Given @vstinner's change in 3.13, the likelihood of getting the same value is slim for 3.13+.

That said, after rereading the method description, I found the wording awkward.

What I'm saying is that the datetime type implies a nanosecond precision/accuracy and the docs never state otherwise.

@spacemanspiff2007, that is a fair point about implying nanosecond precision/accuracy.

Perhaps the best solution would be to add something similar to the description and remove the proposed note: This method is dependent on the precision/accuracy of the operating system's clock, which is typically nanosecond precision on modern systems. If you see unexpected results, consider that older operating systems may have less precision, such as microseconds.

@gvanrossum Does that make sense to you?

@gvanrossum
Copy link
Member

I'm currently trying to find time to really take in everything that is said about this topic in both the datetime and the time module. There are several related concepts, accuracy, precision (not the same thing!), resolution, and the capacity of the representation.

@tim-one
Copy link
Member

tim-one commented Oct 4, 2024

These things are hard. Despite that I wrote the original code, offhand I'm not even sure whether successive calls to .now() return a monotonically non-decreasing sequence of values. Because Python has little to do with it - it's more up to the behaviors of the platform OS and C library functions Python invokes. We're at their mercy.

@tim-one
Copy link
Member

tim-one commented Oct 4, 2024

Noting that it's still quite possible for .now() to repeat values under the released Python 3.12.7 on 64-bit Windows 10 Pro:

>>> from datetime import datetime
>>> now = datetime.now
>>> len(set(now() for i in range(1_000_000)))
492
>>> len(set(now() for i in range(1_000_000)))
488

So, as shown, when going full-bore it's rare not to get a repeated value.

@willingc
Copy link
Contributor

willingc commented Oct 4, 2024

Thanks @tim-one for chiming in and offering your thoughtful perspective.

Because Python has little to do with it - it's more up to the behaviors of the platform OS and C library functions Python invokes. We're at their mercy.

Completely agree, and perhaps all that needs to be said is "the behaviors of the platform OS and C library functions" influence the return value.

@gvanrossum
Copy link
Member

I'm currently trying to find time to really take in everything that is said about this topic in both the datetime and the time module. There are several related concepts, accuracy, precision (not the same thing!), resolution, and the capacity of the representation.

Alas, I don’t think I will find the time any time soon. Someone else might do that review; in the meantime I don’t want to hold up this PR. Maybe @willingc can merge it?

@willingc
Copy link
Contributor

willingc commented Oct 8, 2024

Sure thing @gvanrossum. @spacemanspiff2007 I'm going to merge this, and we can always revisit in the future. Thanks for your PR and patience during the review discussion. ☀️

@willingc willingc merged commit 760b1e1 into python:main Oct 8, 2024
23 checks passed
@willingc willingc added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Oct 8, 2024
@miss-islington-app
Copy link

Thanks @spacemanspiff2007 for the PR, and @willingc for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @spacemanspiff2007 for the PR, and @willingc for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 8, 2024
…n the same instant (pythonGH-124834)

* Update datetime.rst

* Update datetime.rst

replace warning with note

* Update Doc/library/datetime.rst

Co-authored-by: Victor Stinner <[email protected]>

* Update Doc/library/datetime.rst

Co-authored-by: Bénédikt Tran <[email protected]>

---------

(cherry picked from commit 760b1e1)

Co-authored-by: spacemanspiff2007 <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 8, 2024
…n the same instant (pythonGH-124834)

* Update datetime.rst

* Update datetime.rst

replace warning with note

* Update Doc/library/datetime.rst

Co-authored-by: Victor Stinner <[email protected]>

* Update Doc/library/datetime.rst

Co-authored-by: Bénédikt Tran <[email protected]>

---------

(cherry picked from commit 760b1e1)

Co-authored-by: spacemanspiff2007 <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Oct 8, 2024

GH-125145 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 8, 2024
@bedevere-app
Copy link

bedevere-app bot commented Oct 8, 2024

GH-125146 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 8, 2024
willingc pushed a commit that referenced this pull request Oct 8, 2024
…rn the same instant (GH-124834) (#125145)

gh-124832: Add a note to indicate that `datetime.now` may return the same instant (GH-124834)

* Update datetime.rst

* Update datetime.rst

replace warning with note

* Update Doc/library/datetime.rst



* Update Doc/library/datetime.rst



---------

(cherry picked from commit 760b1e1)

Co-authored-by: spacemanspiff2007 <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
willingc pushed a commit that referenced this pull request Oct 8, 2024
…rn the same instant (GH-124834) (#125146)

gh-124832: Add a note to indicate that `datetime.now` may return the same instant (GH-124834)

* Update datetime.rst

* Update datetime.rst

replace warning with note

* Update Doc/library/datetime.rst



* Update Doc/library/datetime.rst



---------

(cherry picked from commit 760b1e1)

Co-authored-by: spacemanspiff2007 <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
efimov-mikhail pushed a commit to efimov-mikhail/cpython that referenced this pull request Oct 9, 2024
…n the same instant (python#124834)

* Update datetime.rst

* Update datetime.rst

replace warning with note

* Update Doc/library/datetime.rst

Co-authored-by: Victor Stinner <[email protected]>

* Update Doc/library/datetime.rst

Co-authored-by: Bénédikt Tran <[email protected]>

---------

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants