gh-120266: Fix undefined _utcfromtimestamp name in datetimetester
#120267
Conversation
| @@ -6223,7 +6223,7 @@ def nondst_folds(self): | |||
| for (_, prev_ti), (t, ti) in pairs(zip(self.ut, self.ti)): | |||
| shift = ti[0] - prev_ti[0] | |||
| if shift < ZERO and ti[1] == prev_ti[1]: | |||
| yield _utcfromtimestamp(datetime, t,), -shift, prev_ti[2], ti[2] | |||
| yield datetime.fromtimestamp(t, datetime.UTC), -shift, prev_ti[2], ti[2] | |||
There was a problem hiding this comment.
Was this causing test failures somewhere? I'm guessing not. It looks like the undefined identifier was introduced last year, in 38dc3f2, so I would have expected failures then. Perhaps we could drop the method entirely. I'm sure @pganssle would have a better idea on what's going on here.
As to the specific fix here, that seems valid. However, sometimes in tests we want to avoid specific code paths for certain test cases. It isn't clear to me if there would be any reason for to avoid calling datetime.fromtimestamp() directly here. If not then this change should be good to go.
Regardless, I definitely agree that it looks like we can't just leave _utcfromtimestamp there. 😄
There was a problem hiding this comment.
Yeah does this mean that nondst_folds is not finding anything? Or is it possible that we're just not using this whole class for something?
In any case, this is not the right thing to do, because it is returning an aware datetime. I think _utcfromtimestamp was supposed to be an alias for something like lambda t: datetime(1970, 1, 1) + timedelta(seconds=t). I am fuzzy on the details now, but I think this kind of thing was one of the rare legitimate uses of utcfromtimestamp.
There was a problem hiding this comment.
nondst_folds is not used at all. Quoting myself:
This problem is not detected by tests, because this looks like a in-test helper for module maintainers. It is not used as of right now.
There was a problem hiding this comment.
In any case, this is not the right thing to do, because it is returning an aware datetime.
Side effects?
nondst_folds is not used at all.
cpython/Lib/test/datetimetester.py
Line 6614 in 8da5ca4
@sobolevn Even here?
zware
changed the title
_utcfromtimestamp name in datetimetested_utcfromtimestamp name in datetimetester
| @@ -6223,7 +6223,7 @@ def nondst_folds(self): | |||
| for (_, prev_ti), (t, ti) in pairs(zip(self.ut, self.ti)): | |||
| shift = ti[0] - prev_ti[0] | |||
| if shift < ZERO and ti[1] == prev_ti[1]: | |||
| yield _utcfromtimestamp(datetime, t,), -shift, prev_ti[2], ti[2] | |||
| yield datetime.fromtimestamp(t, datetime.UTC), -shift, prev_ti[2], ti[2] | |||
There was a problem hiding this comment.
| yield datetime.fromtimestamp(t, datetime.UTC), -shift, prev_ti[2], ti[2] | |
| yield datetime.utcfromtimestamp(t), -shift, prev_ti[2], ti[2] |
There was a problem hiding this comment.
utcfromtimestamp is deprecated.
also cc @ericsnowcurrently who recently reviewed similar PRs in this module :)
_utcfromtimestampis undefined indatetimetester#120266