Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace pytz dependency in tests #3165

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

willthong
Copy link
Contributor

Pull Request Checklist

Resolves: #2958 (something like a further resolution because it now includes a resolution within the tests)

External dependency pytz has already been removed from the main code, but this PR also removes it from the tests.
Why have I changed the test timezone from CET to Europe/London? The tests were written with the W3 examples in mind, but not accounting for the summertime offset. As such, they originally assumed that datetime(year=1997, month=7, day=16, hour=19, minute=20, tzinfo=ZoneInfo("CET")) would evaluate to 1997-07-16T19:20+01:00. But it doesn't; in summertime (July), CET is actually +02:00 and an example of a +01:00 timezone would be "Europe/London". With the shift to the built in ZoneInfo module, the test therefore needs to be adapted.

  • 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

External dependency `pytz` has already been removed from the main code,
but this PR also removes it from the tests. Why have I changed the test
timezone from CET to Europe/London? The tests were written with the [W3
examples](http://www.w3.org/TR/NOTE-datetime) in mind, but not
accounting for the summertime offset. As such, they originally assumed
that datetime(year=1997, month=7, day=16, hour=19, minute=20,
tzinfo=ZoneInfo("CET")) would evaluate to 1997-07-16T19:20+01:00. But it
doesn't; in summertime (July), CET is actually +02:00 and an example of
a +01:00 timezone would be "Europe/London". With the shift to the built
in ZoneInfo module, the test therefore needs to be adapted.
Copy link
Member

@avaris avaris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

Copy link
Member

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks to @willthong for the test enhancement and to @avaris for the review. 💯

@justinmayer justinmayer changed the title Replace pytz dependency in tests Replace pytz dependency in tests Aug 15, 2023
@justinmayer justinmayer merged commit 2eeff62 into getpelican:master Aug 15, 2023
10 checks passed
@willthong willthong deleted the replace-pytz-in-tests branch August 21, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace pytz dependency with zoneinfo
3 participants