Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
GH-103944: Remove last use ofutcfromtimestamp#103995
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
utcfromtimestamputcfromtimestampLib/test/datetimetester.py Outdated
| sign = int(ts / ats) | ||
| ts, frac = divmod(ats, 1.) | ||
| ts *= sign |
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.
We can use math.copysign to copy sign directly from to here for a more efficient implementation.
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.
Yeah, this whole thing was an effort to avoid importingmath, so it's something of a re-implementation ofmath.modf. At the end of the day that seems... pointless, particularly for the test suite, so I just went ahead and usedmath.modf.
Lib/test/datetimetester.py Outdated
| # XXX Copied from test_float. | ||
| INF = float("inf") | ||
| NAN = float("nan") | ||
| def _utcfromtimestamp(klass, ts): |
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.
I wonder why simple timedelta arithmetic will not work here:
def_utcfromtimestamp(klass,ts):returnklass(1970,1,1)+timedelta(seconds=ts)
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.
@pganssle - I ran test_datetime locally with this variant and all test cases pass. I am not sure I can push to your branch, but I will try to submit this change somehow to CI.
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.
Frankly, given that this is only used in tests, I am not sure how much effort is justified in polishing this, but it is probably good to keep test code simple.
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.
Good idea, I was just kinda mindlessly copying the implementation fromdatetime, but usingtimedelta arithmetic is much more elegant.
It's also only used in the one place, so I just inlined it.
There might be easier ways to do this in the future, or we may be ableto avoid it entirely by switching to `ZoneInfo` for these tests, butthis will remove the last valid use of `utcfromtimetamp` that I see.
In one test, we simply turn off DeprecationWarning rather than assertingabout it, because whether the error condition happens before or afterthe warning seems to differ between the Python and C versions.
OK, I think that the DeprecationWarnings might be breaking a buildbot or otherwise making it hard to run the test suite, so I am going to go ahead and merge this. |
Uh oh!
There was an error while loading.Please reload this page.
I guess we missed a few places where
DeprecationWarningwas being raised in the first round of this.This should pass
./python -We -m test -v test_datetimenow.