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. 😄

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.

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.