search

portfolio-performance / portfolio

Track and evaluate the performance of your investment portfolio across stocks, cryptocurrencies, and other assets.
http://www.portfolio-performance.info
Eclipse Public License 1.0
2.77k stars 579 forks source link

Quotes from JSON (Timezones Not Treated As UTC) #2106

Closed metal450 closed 5 months ago

metal450 commented 3 years ago

I'm pulling crypto price quotes from a JSON API. It works, except for one issue: the dates it receives are always "ahead" by a day, thus it's not actually showing the latest quote - it's showing the quote from one day prior.

My assumption is that this is an issue of timezone: although the JSON API is providing standard times in UTC, the software seems to be treating them as "local time." As a result, it thinks the latest quote is one day in the future, and instead displays yesterday's quote for today.

I've experienced this issue with 3 different JSON APIs, so it definitely seems to be on the side of PP. It's true whether the API returns the timestamp as a string (i.e. "2021-02-26T06:19:00Z") or epoch (i.e. "1614297600").

Here's an example of an API you can try as-is, without needing a key:

URL: https://min-api.cryptocompare.com/data/v2/histoday?fsym=EWT&tsym=USD&limit=5
Path To Date: $.Data.Data[*].time
Path To Close: $.Data.Data[*].close

My local system is in Pacific Standard Time. The chart at the bottom of the "Historical Quotes" dialog shows all the correct quotes - but the latest quote is shown as tomorrow's date, and the quote actually used by the software is always one day behind (aka the second quote in the list).

metal450 commented 3 years ago

Addendum: This is true with the newly-added "Binance" quote provider, too. The "Historical Quotes" dialog shows today's quote with tomorrow's date (all dates are shifted forward by a day), and the software uses yesterday's quote instead of today.

ghost commented 3 years ago

Did you have an idea how to solve? There are mostly no UTC zone shift information inside JSON, without this information it's impossible to readjust the date.

metal450 commented 3 years ago

You don't need any zone shift information in the JSON - if we just assume that APIs provide dates in UTC (which to my knowledge, they all do), simply get the current system's timezone & add that as an offset.

ghost commented 3 years ago

This behaviour isn't true, at least Binance and above referred JSON feed has not such information.

metal450 commented 3 years ago

As stated in my last reply, you don't need any information from the API to assume the timestamps they provide is UTC by default. Currently PP seems to assume "local system time," which will clearly not be correct (unless you happen to live in UTC).

Unix Epoch Time is always based on UTC: https://en.wikipedia.org/wiki/Unix_time

metal450 commented 3 years ago

Here's a screenshot to show the issue (this is with the Binance quote provider, it behaves the same as JSON). You can see that today's date is March 1, but PP shows the latest quote as Mar 2, aka in the future. And you can see that its showing yesterday's quote as the "latest" (arrows 3 & 4), but simply ignoring the true latest quote (which it thinks it's in the future). Note: you may not see this issue depending on the time of day (i.e. in my timezone, PST, I only see this issue in the latter half of the day).

2021-03-01 23 04 04

RomanLangrehr commented 3 years ago

For the JSON-API, PP interprets the Unix Epoch Time as „in UTC timezone”, but also outputs the date in UTC (i.e. does no timezone conversion). I guess it would be more intuitive if the output would be in local time, but that might cause some inconsistencies if people change their timezone.

metal450 commented 3 years ago

The issue isn't so much how it's shown on "Historical Quotes," as that dialog isn't really used after initially getting it up and running - the issue is that it doesn't use the most recent quote for the "latest price" (aka see arrow 3 - latest price is coming from yesterday's price, rather than today's). This affects Statement of Assets, etc. With volatile assets like crypto, this can make a huge difference in value from day to day (aka Bitcoin can swing $10k in a day, so by using quotes that are one day earlier, Statement of Assets could be wildly inaccurate).

RomanLangrehr commented 3 years ago

Unfortunately, I still didn’t manage to reproduce the bug. I found an issue by code inspection, but I am not sure if it really solves the issue. Maybe you can help me with the following two points: 1) In your screenshot, on the bottom right it shows the correct date and a price that is similar to the one in the preview. Am I right to assume that the price in the bottom right does get updated but the latest price in the big table does not? 2) Does restarting PP solve the issue?

metal450 commented 3 years ago
  1. You're right - the price in the lower right looks correct, but the price in the 'main ui' (i.e. the big table on All Securities, or on Statement of Assets, etc) is a day behind. Interesting - I hadn't even noticed that price on the lower right, I was just looking on the main Securities chart & Statement of Assets.
  2. Nope, restarting PP doesn't solve it.
metal450 commented 3 years ago

Addendum: now that you've pointed out that the little summary on the lower right is correct, it's quite easy to see which securities the bug occurs for :) Just use the down arrow to scroll through the list. Securities that come from Binance or JSON, that quote on the lower right won't match the main chart. Securities that come from "Search for Assets" work fine - the quote in the lower right does match the quote on the main chart :)

RomanLangrehr commented 3 years ago

I still couldn’t reproduce this. I set my timezone to PST, set up a security with JSON historical quote feed as in your initial post. For me, the latest quote does get updated after UTC midnight and always shows the most recent quote. Do you maybe have set up something odd in the latest quote tab? I.e. is not set to “same as historical quotes”?

metal450 commented 3 years ago

It's definitely set to "same as historical quotes."

Note: you'll only see the discrepancy during the part of the day when the date is different in the two timezones. For example, right now as I write this, at 9:50am PST, you won't see the issue, as the date is 03/17 in both PST and UTC - thus, the Latest Quotes are all correct. You have to wait until a bit later in the evening, after 17:00 (5pm) PST. Between 5pm PST and midnight PST, the date will be 03/17 in PST but 03/18 in UTC. During that interval is when I it shows yesterday's quote as the 'latest.' Then after the clock ticks midnight in PST, it becomes 03/18 in BOTH timezones, and the quotes will again all be correct - until the next evening.

(side note: Another way u can test it, rather than JSON, is with the Binance quote provider - for symbol, try "AAVEUSDT.")

metal450 commented 3 years ago

I've been following all the commits in hopes of a fix for this, & noticed: https://github.com/buchen/portfolio/commit/7a5c74bc55468aa62165358cae2ac18d7e524ae5

Just wanted to let you know that I tried the new 0.51.3, & unfortunately, it seems that didn't fix it. "Latest" shown in the Securities table & elsewhere still are a day behind, vs "Latest Price" in the lower right of the Securities, which is correct.

metal450 commented 3 years ago

I figured out a way to workaround this (on Linux), if/until it gets fixed. If you launch it like:

TZ=UTC /opt/portfolio/PortfolioPerformance

You'll fool it into running as UTC timezone, & then quotes & amounts will show the proper values.

buchen commented 3 years ago

Here's a screenshot to show the issue (this is with the Binance quote provider, it behaves the same as JSON). You can see that today's date is March 1, but PP shows the latest quote as Mar 2, aka in the future. And you can see that its showing yesterday's quote as the "latest" (arrows 3 & 4), but simply ignoring the true latest quote (which it thinks it's in the future). Note: you may not see this issue depending on the time of day (i.e. in my timezone, PST, I only see this issue in the latter half of the day).

Thanks @metal450 for the detailed error description. And the sample data. :+1:

I believe this is what is happening:

I just ran your example data. The latest price comes with a timestamp of 1618704000 which is Sun Apr 18 2021 00:00:00 GMT+0000. As I am located in CEST, I never run into the problem. CEST is 2 hours ahead.

I wonder how to solve this. PP assumes "EOD - end of day" prices. Here we actually have an EOD quote, but it happens to be at midnight.

We could assume that all historical prices are always UTC and convert them - when looking for the latest price - into the local time zone. I wonder though if that then creates a problem for timezones before UTC where a closing quote suddenly moves into the future.

metal450 commented 3 years ago

It sounds to me like the issue is in steps 2 & 3. It downloads timestamps, strips the timezone & stores them as local - then assumes that they're local. Shouldn't the solution just be for PP to store all dates in UTC (rather than as the local timezone of the user running the program)? Then behavior will be consistent, regardless of where the user happened to be when they downloaded the quotes, & regardless of where they happen to be currently (when viewing the app). Stored dates are always UTC, all logic is done according to UTC.

RomanLangrehr commented 3 years ago

I think the cleanest solution would be if PP would internally work in UTC time zone. That means:

  1. the current date (java.time.LocalDate) is always the current date in UTC time zone. (We assume that all quote providers also work in UTC time zone). Since there is no way to convert a “date” (without a time) to a different time zone, the GUI would also display all dates as “UTC-dates”.
  2. Instead of using java.time.LocalDateTime, use java.util.Date (which represents a timestamp, i.e. java.util.Date = java.time.LocalDateTime + time zone information) for timestamps (e.g. for buy and sell orders). Time is, of course, displayed in in the user time zone in the GUI.

I implemented 1. in #2217, but 2. is slightly more difficult because it also affects how timestamps are stored in the XML file and one has to ensure backwards-compatibility. But I think 2. is also important, i.e. if a user moves to a different time zone.

RomanLangrehr commented 3 years ago

We could assume that all historical prices are always UTC and convert them - when looking for the latest price - into the local time zone. I wonder though if that then creates a problem for timezones before UTC where a closing quote suddenly moves into the future.

Time zone conversion would solve the problem for this quote provider, but for quote providers that only provide a date (without time) it is not possible. Given only a date, you could only add one day or subtract one day, but subtracting one day just because you are one hour behind UTC does not make sense.

Actually, I think it would not even solve the problem for this quote provider, because it also provides only one quote per day (and uses the UTC midnight timestamp to represent the entire day), so the “time zone conversion” would effectively subtract an entire day here as well. I don’t think that is a good idea.

metal450 commented 2 years ago

Regardless, it seems like the software should itself definitely store UTC so that it's resilient to changing timezones on the local system (i.e. someone traveling internationally with a laptop).

I revisited this because thread, when the new CoinGecko quote provider was added, I tried changing all my Binance ones just on the off chance that somehow it might work. But unfortunately, it still doesn't - I still get one-day old quotes whenever I use the software in the latter half of the day, which (due to the volatility of crypto), leads to balances that can be inaccurate by many thousands of dollars :/

Ricent82 commented 2 years ago

I have this problem too - balances are very far off whenever I use the software in the evening, it seems like it's using the wrong quotes. But it's only like that in the evening, in the morning, it uses the right quotes. that Any hope to fix?

metal450 commented 2 years ago

I've resorted to changing my system clock back & forth to UTC every time I use PortfolioPerformance. Unless this gets fixed, that's the only way I could think of to get accurate quotes.

tquellenberg commented 2 years ago

Is this fixed with #2622 ?

flywire commented 2 years ago

[quote="AndreasB, post:4, topic:19535"]

(I am not aware of any change in that code in the January 2022 version [0.56.3] of PP)

Yes the Historical Quotes in Version: 0.56.2 (Dec. 2021) omit Friday and Saturday for Aus instead of Saturday and Sunday. I can't check much more on a weekend.

The thing I initially noticed was the Performance dashboard Last Day: Absolute change was wrong and I thought it had been reliable.

metal450 commented 2 years ago

Is this fixed with #2622 ?

Not fixed. That PR seems to just specifically address Yahoo, but quotes from other providers (i.e. Coingecko) still don't have the proper timezone, and thus still require changing the system back & forth to be accurate. Running PP in the latter half of the day is still using day-old quotes as though they're the most recent.

buchen commented 2 years ago

To be honest, I am struggling how to address this issue.

What happens?

Most quote provider (for stock prices and funds) only provide a date - no time, no timezone. It is all "end of day" prices in terms when the exchange closed. For example, if it is Xetra prices, the time might be 17:30 CET or of the last trade before the end of the trading hours.

Also keep in mind that none of the calculations know anything about "today" - the input is always a date which may or may not be today. The method SecurityPrice getSecurityPrice(LocalDate requestedDate) is used for all lookups. Let's say, the user wants to have the statement of assets of December 31st. If PP treats all dates as UTC dates at 23:59:59 (at the end of the day), what time to use for the lookup? UTC 23:59:59 -> then it behaves just like today. the local time -> then it depends on the time of the day which historical price is picked up. That would mean the historical valuation of assets depends on the time of day the user does the report. Maybe the calculations need to learn about "today" after all.

What about this procedure:

As an example:

Thoughts?

flywire commented 2 years ago

I haven't worked through that example for Australia but it looks pretty reasonable. A few comments.

Most of Eastern Australia uses the following timezones:

The ASX opens at 10am and has a 20min delay on prices although I think it takes a little longer to be reported by Yahoo. At 11:20 (AEDT) PP reports prices for day before and by 12:00 (AEDT) it reports current days prices.

The workaround I've adopted is don't use daily change reporting, or save any prices on Mondays or before 12:00 AEDT.

The latest price is treated as UTC and the time is stripped

This is where it can change the latest available price for the day from Yahoo to the previous day affecting all daily price change calculations.

  1. The price needs to be converted back to the local time
  2. It's still not satisfactory for Australian users, UTC would bring yesterday's US prices into today which is contrary to reports elsewhere and very important because the US market has a big impact on the Aus market. The AUS day probably changes at 08:00 (AEST/AEDT) depending on current US and NZ timezones, varying by summertime changeover dates.
metal450 commented 2 years ago

Most quote provider (for stock prices and funds) only provide a date - no time, no timezone.

For what it's worth, I did have this issue with every crypto quote provider I've tried: Binance, JSON (using 2 different APIs), CoinGecko, etc. CoinGecko is just where I landed.

What about this procedure:

Seems logical enough. I don't really think I have enough domain knowledge to offer much foresight as to what gotchas might arise, though - the devil is likely in the details, as there are so many usages of dates/times that I might not be aware of. It also sounds like a fairly sweeping change. While a "global" solution to address all quote providers would certainly be very welcome, my minor PR was more of a simple attempt to band-aid the situation, if or until such a broader redesign like this might be feasible (from those with a bit more experience & domain knowledge than myself).

Basically, I'm just a bit desperate to get it working, as having to set my clock back & forth every day for the past year is indescribably cumbersome ;) This was the best quick fix I could come up with, just to tide things over for the moment. Is that fair? :)

buchen commented 2 years ago

as having to set my clock back & forth every day for the past year is indescribably cumbersome

That is an argument. I have now merged your short-term fix. I have slightly edited the change to a) only apply to the latest prices (in order to not the move the historical prices by a date) and b) avoid that the resulting list has two prices for the same date

I believe the short-term fix should not change the behavior for Australian users. You are (just as I am) before UTC. On the date boundary, it moves only the latest quote up by one day (that will be overwritten in later updates with the end of day closing).

For the long term fix outlined above, I need more time (also to get the storage format right).

metal450 commented 2 years ago

Great, thanks!!

flywire commented 2 years ago

I haven't had an opportunity to check PP between UTC 00:20 and UTC 01:00 (ref). Regardless, Version: 0.56.4 (Jan. 2022) seems to have fixed all reporting date issues for Australia. It passes Sanity Check, fairly sure it didn't even need an Update Quotes.