Replace pytz dependency with zoneinfo. Fix #2958

[willthong]

Pull Request Checklist

Resolves: #2958

• Ensured tests pass and (if applicable) updated functional test output
• Conformed to code style guidelines by running appropriate linting tools
• Added tests for changed code
• Updated documentation for changed code

Also wasn't sure how to add tests for changed code - sorry.

[justinmayer]

Hey Will. Many thanks for the contribution. There seems to be one or more CI failures with tests and linters. Would you be so kind as to take a look and see what needs to be done to fix those?

[willthong]

Apologies, I've fixed the linting error but am stuck on the Python 3.8 test! Here's what I've done to troubleshoot:

• Locally, checked that Poetry is at the right version (poetry env info returns Python 3.8.17 as expected)
• Run poetry install to make sure packages are downloaded per the pyproject.toml file
• Run poetry show to verify that backports-zoneinfo 0.2.1 is installed as expected
• Run invoke tests, which confirms that on Python 3.8.17, all 264 tests passed

Would be grateful for any pointers as to why the test is failing on the server but not locally!

[justinmayer]

Let me know if you have any trouble fixing the import order that is causing the linter conformance failure.

The solution to the "works locally but not in CI" mystery is that we haven't yet fully switched over to pyproject-based builds, which means the canonical dependencies are still defined in setup.py, so the CI environment uses python -m pip install -e . instead of poetry install. The latter will install the dependency you added but the former will not. Sorry for the confusion. Eventually we will get around to fully switching and then removing setup.py altogether.

In the interim, I rectified that problem by adding backports-zoneinfo to setup.py via commit 85cb041 to this PR's branch.

With that problem fixed, the next issue has reared its head, which is a test failure in the Windows test environment. Could you take a look at the test failure output and see what you can come up with to fix that failure?

[willthong]

Thanks @justinmayer! Useful to know. I think maybe maintainers need to approve tests so I'm not completely sure, but I believe the Windows problem may be because Windows doesn't have an IANA database so I should have used backports-zoneinfo[tzdata] to retrieve a PyPi version of that database. As for the linting error, that seems to have been a misordering of dependencies and I believe I've now fixed it.

[justinmayer]

Thanks for fixing (hopefully) the Windows test failure.

By the way, it's usually not a good idea to add formatting changes to a PR that aren't related to changed lines of code. In this case, for example, there are a ton of changed lines in this PR that are probably the result of running Black, et al, on entire file(s). Those kinds of widespread formatting changes are usually best done in the context of separate PRs. Otherwise it's too hard to see the actual relevant code changes, because they are lost in a sea of formatting changes.

So my suggestion would be to replace the last commit with one that just addresses problems that are fixed by pre-commit, as that tool should only operate on changed lines of code.

[willthong]

Thank you! I realised that I need to turn off my Black linter after I'd pushed, so I'll remember to avoid doing so next time. I ought also to check the diff to make sure I'm not inadvertently pushing more changes than I expect.

Hopefully the final commit with linting and substantive tests all passing now.

[justinmayer]

Many thanks for your work on this, Will. Nicely done! Tests are all now passing, and I don't see any glaring problems jumping out at me.

@avaris: Would you take a quick look at this, just as a second pair of eyes?

[willthong]

Brilliant, thanks Justin! I am conscious that the tests still depend on pytz - if helpful, I'd be happy to remove this dependency (perhaps as a separate PR).

[justinmayer]

Many thanks for that offer, Will. Removing pytz as a dependency in the tests would indeed be helpful, and a separate PR seems like a good place to do that.

If @avaris doesn't have time to chime in within the next few hours, I'll go ahead and merge this so we can proceed. Any needed follow-up adjustments can be done "in post", as it were. 😊

[justinmayer]

Would you please make the same improvement noted above in the other two relevant files in this PR? That is:

• pelican/tools/pelican_quickstart.py
• pelican/utils.py

[willthong]

Thanks! Grepped for all zoneinfo references and all done.

[justinmayer]

Many thanks to @willthong for the enhancement and to @avaris for reviewing! 🌟