time-machine
time-machine copied to clipboard
Feature/warn when using pytz
As detailed in #325 : I would do something like this here.
I've added a print-out of the webpage in a docs/ folder. Just in case that it disappears from the internet (have had too many references disappear that way).
One thing: I couldn't run the tests on my windows machine because in conftest you have time.tzset(). I couldn't disable this without bringing the coverage to 99%; thus failing the workflow.
You don't encounter this issue because the workflow only tests on ubuntu. But it was a bit annoying 🤪.
My suggestion would be to remove conftest from the coverage report (as there's nothing in there anyway), and do something like I did in commit ddea169ca8283c47c574ffc1a6a3e8e875b20d99:
if hasattr(time, 'tzset'): # Not available on Windows
time.tzset()
Another way i tried was:
if sys.platform != "win32":
time.tzset()
Anyway, minor annoyance only. Though would be nice to run the tests locally.
One other thing: only your main branch builds tests. There is a case to be made to let any branch test. WDYT?
@adamchainz , how to fix the precommit hook exactly?
I've added a print-out of the webpage in a
docs/folder. Just in case that it disappears from the internet (have had too many references disappear that way).
Please remove and instead ensure Paul’s site is added to the way back machine: https://archive.org/ . If it goes offline, we'll use that.
But don't worry, I'm sure Paul's site will be online for a while. I have a broken link checker running on my blog, and several times I've spotted Paul's certificate expiring and pinged him to fix it :)
One thing: I couldn't run the tests on my windows machine because in conftest you have
time.tzset(). I couldn't disable this without bringing the coverage to 99%; thus failing the workflow.You don't encounter this issue because the workflow only tests on ubuntu. But it was a bit annoying 🤪.
Happy to try with your suggestion if it makes the tests work on Windows locally. I don't think running tests on Windows on CI is worth the extra wait times right now, if it's just that tzset that's the problem.
One other thing: only your main branch builds tests. There is a case to be made to let any branch test. WDYT?
Tests run when you open a PR. If you want to run the tests on CI, open a PR. You can make it a draft if you don't want to ping me.
If you set tests to run on any branch, they run twice on each commit to a PR, wasting time/energy.
Please remove and instead ensure Paul’s site is added to the way back machine: https://archive.org/ . If it goes offline, we'll use that.
A snapshot was captured. Visit page: /web/20230215195634/https://blog.ganssle.io/articles/2018/03/pytz-fastest-footgun.html
I've another question: What about this:
elif HAVE_PYTZ and isinstance(dest.tzinfo, pytz.BaseTzInfo):
if sys.version_info[:2] >= (3, 9):
dest = dest.replace(tzinfo=zoneinfo.ZoneInfo(dest.tzinfo.zone))
else:
raise TypeError(
"We don't support pytz. For more background information, please "
"read the pytz topic in the readme."
)
I'm not quite certain it's the right way to do. Because the datetime might be already eagerly calculated, and then changing the timezone information will mess it up... So maybe erroring out completely is the safer way. Just thought I'd propose it here :)