GH-103944: Remove last use of utcfromtimestamp
#103995
Conversation
pganssle
changed the title
utcfromtimestamputcfromtimestamp
Lib/test/datetimetester.py
Outdated
| sign = int(ts / ats) | ||
|
|
||
| ts, frac = divmod(ats, 1.) | ||
| ts *= sign |
There was a problem hiding this comment.
We can use math.copysign to copy sign directly from to here for a more efficient implementation.
There was a problem hiding this comment.
Yeah, this whole thing was an effort to avoid importing math, so it's something of a re-implementation of math.modf. At the end of the day that seems... pointless, particularly for the test suite, so I just went ahead and used math.modf.
pganssle
force-pushed
the
utcnow_deprecation
branch
from
6759ba2 to
82ac549
Compare
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.
I wonder why simple timedelta arithmetic will not work here:
def _utcfromtimestamp(klass, ts):
return klass(1970, 1, 1) + timedelta(seconds=ts)There was a problem hiding this comment.
@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.
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.
Good idea, I was just kinda mindlessly copying the implementation from datetime, but using timedelta 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 able to avoid it entirely by switching to `ZoneInfo` for these tests, but this will remove the last valid use of `utcfromtimetamp` that I see.
In one test, we simply turn off DeprecationWarning rather than asserting about it, because whether the error condition happens before or after the warning seems to differ between the Python and C versions.
pganssle
force-pushed
the
utcnow_deprecation
branch
from
82ac549 to
de0a954
Compare
|
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. |
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.