ranaroussi / yfinance

Download market data from Yahoo! Finance's API
https://aroussi.com/post/python-yahoo-finance
Apache License 2.0
14.93k stars 2.45k forks source link

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

Closed mreiche closed 3 months ago

mreiche commented 3 months ago

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.

ValueRaider commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

Approved. Just squash the commits #1084

mreiche commented 3 months ago

Approved. Just squash the commits #1084

Can't you just enable squash commits in your repo? https://docs.github.com/de/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests

ValueRaider commented 3 months ago

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?