Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-59648: Nanosecond support for datetime#92078
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
bedevere-bot commentedApr 30, 2022
Every change to Pythonrequires a NEWS entry. Please, add it using theblurb_it Web app or theblurb command-line tool. |
MojoVampire left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Summary:
- Don't apply automatic style fixes; it bloats the patch and makes it much harder to review
- Using
Decimal
the way you're using it is inherently unsafe due to unknown contextual precision, and will make the code slower to boot. Find a way to do your work withint
when usingfloat
becomes impossible. Bonus: It won't make importingdatetime
force you to loadDecimal
in the process (a relatively heavyweight module) - Several comparison and hash functions aren't using the submicrosecond data, so the extra data gets ignored, which seems like a problem.
- You broke pickling. Please don't do that.
- (A biggy I couldn't comment on inline) This only changes the Python level
datetime
module, that CPython itself doesn't even use, by and large. Any change to the Python level code indatetime.py
requires a complementary change in_datetimemodule.c
so the C accelerated version of the module has the same behavior (you may have avoided at least a few test failures because they were testing the C module and didn't notice the broken code in the Python module)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This API looks reasonable to me.@tim-one What do you think? |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Adds nanosecond as a keyword argument for backward compatibility
Solves:#59648