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 with zoneinfo (backports.zoneinfo for Python 3.6–3.8) #614

Closed
wants to merge 1 commit into from
Closed

Conversation

eumiro
Copy link
Contributor

@eumiro eumiro commented Jan 8, 2021

Python 3.9 brings zoneinfo in its stdlib and it is also distributed as backports.zoneinfo for Python 3.6–3.8. Replace pytz with it.

  • Tests with a significant number of years to be tested for your calendar.
  • Changelog amended with a mention describing your changes. Note Please do NOT change the version number here. It's the project maintainers' duty.

setup.cfg Outdated
@@ -30,7 +30,7 @@ install_requires =
skyfield-data
python-dateutil
lunardate
pytz
backports.zoneinfo
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it's worth making it a dependency only when needed or it this will create more problems than it will solve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great hint. So something like

backports.zoneinfo;python_version<"3.9"

Copy link
Member

Choose a reason for hiding this comment

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

can you enlight me here? Even if pytz is not in the stdlib, it works, doesn't it? And if you want to support Python 3.6 to 3.9, it looks like the only Python version where zoneinfo is in the stdlib is the 3.9, so it's not "widely standard".
In a word or two, why would we have to switch to zoneinfo? Is there a definitive advantage to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ewjoachim I updated it and repushed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brunobord zoneinfo uses your system's tz database, while pytz sometimes brings its own.

brunobord added a commit that referenced this pull request Feb 19, 2021
**Major changes**

- API: New method available in `core` module: `Calendar.get_iso_week_date()` to find the weekday X of the week number Y (#619).
- Requirements: Replace pytz with `(backports.)zoneinfo`, thx to @eumiro (#614)
- Doc: Documented the different (in)compatibilities due to the use of `zoneinfo` (#614).

**Minor changes**

*Bugfix*

- Small fixes in Netherlands School calendars (#619).
- Temporary downgrade of `pyupgrade` to fix the `pyup_dirs`.

*Improving test coverage*

- Improve Netherlands coverage (#546, #619).
- Improve Russia coverage (#546).
- Improve USA calendar coverage by removing a method that wasn't used anyways (`get_washington_birthday_december()`). The method is implemented in both Indiana and Georgia State calendars, and is specific for each state, even if they look very similar (#546).
- Improve the `astronomy.py` module coverage (#546).
- Improve coverage for the `tests/__init__.py` module (#546). *Note:* system-dependant test branch (if Windows) won't be counted for coverage.
@brunobord brunobord mentioned this pull request Feb 19, 2021
14 tasks
brunobord added a commit that referenced this pull request Feb 19, 2021
**Major changes**

- API: New method available in `core` module: `Calendar.get_iso_week_date()` to find the weekday X of the week number Y (#619).
- Requirements: Replace pytz with `(backports.)zoneinfo`, thx to @eumiro (#614)
- Doc: Documented the different (in)compatibilities due to the use of `zoneinfo` (#614).

**Minor changes**

*Bugfixes*

- Small fixes in Netherlands School calendars (#619).
- Temporary downgrade of `pyupgrade` to fix the `pyup_dirs`.

*Improving test coverage*

- Improve Netherlands coverage (#546, #619).
- Improve Russia coverage (#546).
- Improve USA calendar coverage by removing a method that wasn't used anyways (`get_washington_birthday_december()`). The method is implemented in both Indiana and Georgia State calendars, and is specific for each state, even if they look very similar (#546).
- Improve the `astronomy.py` module coverage (#546).
- Improve coverage for the `tests/__init__.py` module (#546). *Note:* system-dependant test branch (if Windows) won't be counted for coverage.
brunobord added a commit that referenced this pull request Feb 19, 2021
**Major changes**

- API: New method available in `core` module: `Calendar.get_iso_week_date()` to find the weekday X of the week number Y (#619).
- Requirements: Replace pytz with `(backports.)zoneinfo`, thx to @eumiro (#614)
- Doc: Documented the different (in)compatibilities due to the use of `zoneinfo` (#614).

**Minor changes**

*Bugfixes*

- Small fixes in Netherlands School calendars (#619).
- Temporary downgrade of `pyupgrade` to fix the `pyup_dirs`.

*Improving test coverage*

- Improve Netherlands coverage (#546, #619).
- Improve Russia coverage (#546).
- Improve USA calendar coverage by removing a method that wasn't used anyways (`get_washington_birthday_december()`). The method is implemented in both Indiana and Georgia State calendars, and is specific for each state, even if they look very similar (#546).
- Improve the `astronomy.py` module coverage (#546).
- Improve coverage for the `tests/__init__.py` module (#546). *Note:* system-dependant test branch (if Windows) won't be counted for coverage.
@brunobord
Copy link
Member

the new requirement handling of timezone libraries is now in Workalendar 15.0.0.
If anybody has got any issue with Windows and the new requirements, please do not hesitate to open an issue.

Happy upgrading to anyone!

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.

None yet

3 participants