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

Quote constructor loses timezone information #409

Open
goulashsoup opened this issue Dec 31, 2024 · 4 comments · May be fixed by #410
Open

Quote constructor loses timezone information #409

goulashsoup opened this issue Dec 31, 2024 · 4 comments · May be fixed by #410
Assignees
Labels
bug Something isn't working

Comments

@goulashsoup
Copy link

What happened?

When passing a datetime object with set timezone, the timezone information is gone when using the Quote.date property, leading to "unexpected" results.

I guess this is due to the fact that Quote uses the C# variant internally and this information is then lost somehow.

I still consider it a bug (likely easily fixable), since I don't expect information passed to be suddenly missing afterwards.

Code usage

from stock_indicators.indicators.common.quote import Quote
from decimal import Decimal
from datetime import datetime, timezone

dt = datetime.fromisoformat('2000-03-26 23:00+0000')
print(str(dt.tzinfo)) # UTC
print(str(dt.time())) # 23:00:00

q = Quote(
    date=datetime.fromisoformat('2000-03-26 23:00+0000'), 
    open=Decimal('23'),
    high=Decimal('26'),
    low=Decimal('20'),
    close=Decimal('25'),
    volume=Decimal('323')
)

print(str(q.date.tzinfo)) # None
print(str(q.date.time())) # 01:00:00

Log output

UTC
23:00:00
None
01:00:00
@DaveSkender
Copy link
Member

DaveSkender commented Jan 1, 2025

This is worth a longer conversation to understand and consider. Let's continue discussing here:

I've left some initial thoughts there.

@DaveSkender
Copy link
Member

Try normalizing your dates to UTC format with .astimezone(timezone.utc).

from stock_indicators.indicators.common.quote import Quote
from decimal import Decimal
from datetime import datetime, timezone

dt = datetime.fromisoformat('2000-03-26 23:00+0000')
print(str(dt.tzinfo))  # UTC
print(str(dt.time()))  # 23:00:00

# Ensure the datetime retains its timezone information
q = Quote(
    date=datetime.fromisoformat('2000-03-26 23:00+0000').astimezone(timezone.utc),
    open=Decimal('23'),
    high=Decimal('26'),
    low=Decimal('20'),
    close=Decimal('25'),
    volume=Decimal('323')
)

print(str(q.date.tzinfo))  # UTC
print(str(q.date.time()))  # 23:00:00

DaveSkender added a commit that referenced this issue Jan 1, 2025
Fixes #409

Modify the `DateTime` class in `stock_indicators/_cstypes/datetime.py` to use `datetime.isoformat(timespec='seconds')` to retain timezone information.

Update the `to_pydatetime` function in `stock_indicators/_cstypes/datetime.py` to handle timezone information correctly.

Add a test in `tests/common/test_cstype_conversion.py` to verify that the `Quote` constructor retains timezone information.
* Add `test_quote_constructor_retains_timezone` method to test the `Quote` constructor with timezone-aware datetime objects.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/facioquo/stock-indicators-python/issues/409?shareId=XXXX-XXXX-XXXX-XXXX).
@DaveSkender DaveSkender linked a pull request Jan 1, 2025 that will close this issue
@goulashsoup
Copy link
Author

goulashsoup commented Jan 1, 2025

Try normalizing your dates to UTC format with .astimezone(timezone.utc).

@DaveSkender As you probably found out yourself, this fix doesn't work, but you already created a PR that fixes that, so THANK YOU! 🙂

@DaveSkender DaveSkender self-assigned this Jan 1, 2025
@DaveSkender DaveSkender moved this from 💡 Triage to 🏗 In progress in Stock Indicators for Python Jan 1, 2025
@DaveSkender
Copy link
Member

As you probably found out yourself, this fix doesn't work, but you already created a PR that fixes that, so THANK YOU! 🙂

So far, that experimental PR (#410) isn't working either. Date handling strategy is almost always a game of trade-offs since there are so many inconsistencies across ecosystems and languages. I'm not entirely sure yet that we'll be able to handle every situation internal to the library yet -- just experimenting for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 🏗 In progress
Development

Successfully merging a pull request may close this issue.

2 participants