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

Fix datetime conversion with mixed timezones when ignore_tz is False #2016

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

mreiche
Copy link
Contributor

@mreiche mreiche commented Aug 7, 2024

When querying multi symbols at the same time, the Pandas Timestamp to datetime64 conversion will fail, when utc=False.
It looks like this is the most stable implementation, because Pandas suggests to use this feature in the future.
Downside: The original timezones are lost. But every datetime has now tz=timezone.utc
Upside: ignore_tz may be obsolete.

I added a test for testing Pandas datetime conversion as well.

tests/test_prices.py Outdated Show resolved Hide resolved
@ValueRaider
Copy link
Collaborator

Downside: The original timezones are lost. But every datetime has now tz=timezone.utc

That's fine as long as 9:30am EST is aligned with 2:30pm EU (or whatever the Atlantic offset is). ignore_tz means 9:30am EST aligned with 9:30am EU.

@mreiche
Copy link
Contributor Author

mreiche commented Aug 8, 2024

Downside: The original timezones are lost. But every datetime has now tz=timezone.utc

That's fine as long as 9:30am EST is aligned with 2:30pm EU (or whatever the Atlantic offset is). ignore_tz means 9:30am EST aligned with 9:30am EU.

That's covered by the test TestPandas.test_mixed_timezones_to_datetime() where the datetime64 utc timestamps are compared to the tz-aware Timestamps.

@ValueRaider
Copy link
Collaborator

Approved. Just squash the commits #1084

@mreiche mreiche changed the title Fix datetime conversion with mixed timezones when ignore_tz is not False Fix datetime conversion with mixed timezones when ignore_tz is False Aug 9, 2024
@mreiche
Copy link
Contributor Author

mreiche commented Aug 11, 2024

@ValueRaider
Copy link
Collaborator

Hmm. Could, but then the squashed commit message lists each individual commit e.g. Add assertion for timezone. Why would you not want to rewrite the message?

@mreiche mreiche force-pushed the bugfix/mixed-tz-conversion branch from 5bee777 to f13ff4b Compare August 12, 2024 05:40
@ValueRaider ValueRaider merged commit 459d5f6 into ranaroussi:main Aug 12, 2024
1 check passed
@ValueRaider ValueRaider mentioned this pull request Aug 22, 2024
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.

2 participants