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

Migrate from pytz to zoneinfo #329

Merged
merged 3 commits into from
Sep 9, 2023
Merged

Migrate from pytz to zoneinfo #329

merged 3 commits into from
Sep 9, 2023

Conversation

maread99
Copy link
Collaborator

@maread99 maread99 commented Sep 7, 2023

Migrates code base and tests from pytz to zoneinfo.

Also updates dependencies.

Migrates code base and tests from `pytz` to `zoneinfo`.
@maread99 maread99 added dependencies Pull requests that update a dependency file maintenance labels Sep 7, 2023
@maread99 maread99 marked this pull request as draft September 7, 2023 08:23
@maread99
Copy link
Collaborator Author

maread99 commented Sep 7, 2023

Tests are passing in 3.9. A single test, test_trading_index, is failing in 3.11. I'm unable to recreate the failure in a local 3.11 environment so I'm debugging via raising errors and inspecting the output in the GitHub workflow console (I can't get simple print statements to appear there??). Hence all the forced pushes.

The failure's odd. Something is resulting in the tz of the expected index to be set to a pytz timezone, but only in 3.11. and not in my local 3.11 environment. 🤷

@maread99
Copy link
Collaborator Author

maread99 commented Sep 7, 2023

The issue was with pandas.read_csv. 'Usually' this uses datetime.timezone.utc to represent the UTC timezone, although it would appear that pytz.UTC is used for certain combinations of pandas and python, including at least pandas 3.1.0 / py 3.11.5 (but not pandas 3,1.0 / py 3.11.0 or pandas 3.1.0 / py 3.9.13). Who knows what's going on under-the-bonnet.

Drops `pytz`, adds `tzdata`, to `pyproject.toml`.

Pins latest deps on requirements files.
Always converts tz of columns of return from `pandas.read_csv` to
`timezone.ZoneInfo("UTC")` (some combinations of pandas and python
will return as `pytz.UTC`).
@maread99 maread99 marked this pull request as ready for review September 7, 2023 14:22
@maread99
Copy link
Collaborator Author

maread99 commented Sep 7, 2023

@gerrymanoim this is good to go. If you're happy with it I'll merge it (and the other PR that's come in) and cut 4.4.

Closes #322.

Cheers.

@gerrymanoim
Copy link
Owner

LGTM.

Did you do this manually? I've had good success using https://github.com/isidentical/refactor to do these types of large scale changes fyi.

@maread99
Copy link
Collaborator Author

maread99 commented Sep 7, 2023

package-wide Find and Replace for most of it and then just manually tidied up where usage was a little different. A lot less painful than I was expecting. Spent three times as long unearthing the inconsistent behaviour from pd.read_csv, a pita.

I'll merge and release tomorrow.

Thanks for the head up with the refactor package, I'll check it out.

@maread99 maread99 merged commit 97716a0 into master Sep 9, 2023
8 checks passed
@maread99 maread99 deleted the tz branch September 9, 2023 00:06
@maread99
Copy link
Collaborator Author

maread99 commented Sep 9, 2023

Anticipate releasing under 4.4 early next week (w/c 23/09/11)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants