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

YTD - Year to Date - Incorrect Percent Compare - Highstock #1613

Open
deadwaxpoet opened this Issue Mar 21, 2013 · 26 comments

Comments

@deadwaxpoet

deadwaxpoet commented Mar 21, 2013

Hi your charting software is fantastic, much love & respect! However I've come across an issue.

When using the YTD range button, Highstock is plotting from Jan 2nd forward, throwing percent compare off. For financial services the accepted method is from DEC 31st. DEC 31st of the prior year should be 0.00% starting point.

i.e. Dec 31 2012 to current date

Is there a quick solution to this?

@TorsteinHonsi

This comment has been minimized.

Collaborator

TorsteinHonsi commented Jun 19, 2013

Related to #687.

@TorsteinHonsi

This comment has been minimized.

Collaborator

TorsteinHonsi commented Jun 19, 2013

We're discussing a fix for this. How would you expect the start date to be if the user selects say May 1st through May 31st? Would the starting point then be the last trading day of April?

@thasmo

This comment has been minimized.

thasmo commented Nov 12, 2014

We're having the same problem with YTD. Calculation should start from December 31st the year before, not on January 2nd. Is there a quick way to set the timeframe manually?

@thasmo

This comment has been minimized.

thasmo commented Nov 12, 2014

@highslide-software, I'm no expert regarding financial services and related topics but I think when manually selecting a date like May 1st, it should use May 1st for calculation. That speaking, YTD is just a shortcut for Dec 31st of the prior year, instead of Jan 1st/2nd.

@craigxgibbons

This comment has been minimized.

craigxgibbons commented May 24, 2016

We are also experiencing this issue. The YTD range doesn't calculate correctly.

+1

@juicelink

This comment has been minimized.

juicelink commented Aug 15, 2017

I use highstock for a financial website. But my customers are not cumfortable with it. When they use other tools they select a range and the compare value start at 0 for the 1st point. If they want performance for a month, they start their range the last day of the previous month. if it's for a week they start their range a day before too. Same thing for YTD. So the feature would not be just for YTD but for every range. Starting point should be at 0 and not with a performance calculated from a previous point not shown. YTD range should have start point at 31 dec (with 0 performance). Month range should select a range of 1month + 1 day and starting point should be at 0. Same thing for week, year and other ranges.

I'm just a developer but it's what I understood from my financial expert. Having that stuff with a financial switch would be awesome. Otherwise, sadly, I'll have to look for a chart library more adapted to financial data.

Her is my financial expert complaint:
If I need to check the performance of the S&P 500 and Nasdaq during july 2017 I should enter 06/30/17 and 07/31/17 to get it. On highcharts, I get the indices which are not properly rebased on the 06/30/17 and performance is wrong...because it seems it takes as a basis the day before! Same if I use my mouse to cover a specific period, the rebasing is wrong and I can't see the first point of it, which is very uncomfortable...
There is just one proper way to calculate a performance during a period.

@TorsteinHonsi

This comment has been minimized.

Collaborator

TorsteinHonsi commented Aug 15, 2017

@pawelfus There seems to be two issues here, one with the compare logic and one with YTD. What do you think? Should we add this as options, or even without options, as there is just "one proper way"?

@juicelink

This comment has been minimized.

juicelink commented Aug 15, 2017

My financial expert says there's "one proper way" in his context (manipulating financial data, calculating performances). Perhaps in other contexts it doesn't fit.
the different options are:
comparison:

  • do we start the comparison from the first point selected or from the previous point?
  • if we select the previous point, do we display it ?
  • range buttons : do we include the previous day in the period selected?

in my (financial) context.

  • to calculate a performance on a period the previous day of the period must be included.
  • financial customers know that and they use to include the previous day when they calculate a performance. ex: select from jan 31 to feb 28 to calculate feb performance.
  • from an user experience when you compare performance it's very disappointing not to have all the curves starting at the same point (0). You always doubt if the different data sets have been correctly rebased. As for us the first point should be the date at which series should be rebased.
  • always from a user experience it's easier to understand that the calculations are done on the points you have selected and that are shown, than on another point out of range, shown or not.It's easier for them to verify if the calculations are exact, and it secures them

They are no bugs on the actual version, It gives the right comparison information when you know how to use and read the graph. For me when I use it I see no issue. But this manner seems not natural and not appropriate for financial using.

I would vote for option as highstock should be used in other contexts, where actual version should fit better.

  • 1st option : the day previous is added to the periods when calculating comparisons. The rebase is done on this hidden previous point. (actual version)
  • 2nd option: No point added for comparison calculations. Calculation are done only on visible points. First point and last point correspond to from and to fields. The rebase is done on first point, that is at 0. The range buttons include the previous point. Ex: 1month set "to" field at a date, and the "from" field at the same date minus one month and one day. That for every button -> with same logic YTD would set from at dec 31 and to at today. It could be done by only switching the range buttons behavior (extend the range with the point before) when the chart is in comparison mode (and of course doing comparison calculations on only selected range)

I wanted to use the different hooks to adapt the behavior, but it seems pretty hard as all must be consistent: from and to date fields, range button , selection by mouse click + rebasing from starting point and not point previous date range...

@pawelfus

This comment has been minimized.

Contributor

pawelfus commented Aug 16, 2017

Hi @juicelink

Thank you for the details, we really appreciate this!

@TorsteinHonsi I agree, we have two issues here, and both should be resolved. I would suggest:

  • add two options to each range button, which can modify calculated min/max extremes (like offsetMin and offsetMax). That way everyone can define how YTD, 1M etc. should work: YTD can start as 31st of Dec, 1st of Jan or 1st of Apr. For months it's even more suitable: 1M can be defined as 1st to 1st, 1st to last-of-month, last-of-previous-month to last-of-the-current-month. 1M issue is not mentioned here, but we had reports for that (as in #5748).

  • compare issue - nothing really to add here. We should add this compare option to define if base calculations should start at the first point of the range, or the first point before the visible range.

Combining these two options user will be able to define, for example for YTD:

  • compare starts at 31st Dec, 31st in visible range with 0%
  • compare starts at 1st Jan, 1st in visible range with 0%
  • compare starts at 31st Dec, 31st not in visible range with 0%, 1st of Jan has already change calculated and is a first point displayed on the chart (current behaviour)

Modification of extremes (offsetMax, offsetMin) can be easily implemented in clickButton(). For a compare modification, I guess we can check #687 commit and improve it.

@TorsteinHonsi

This comment has been minimized.

Collaborator

TorsteinHonsi commented Aug 17, 2017

Great. We'll go with the offsetMin and offsetMax for the buttons then.

As for compare, do we need a new option, like compareBase? Probably that would be needed when the user is not using the range selector buttons, but types in dates manually or moves the navigator handles?

@pawelfus

This comment has been minimized.

Contributor

pawelfus commented Aug 17, 2017

compareBase shouldn't change extremes, but should change which point is taken into calculations in compare mode. At this moment first point within visible range has already calculated the change. The idea is to render first point as base (0.00% change). Compare first point in the tooltip:

@TorsteinHonsi

This comment has been minimized.

Collaborator

TorsteinHonsi commented Aug 18, 2017

Okay, so we need both of these modifications, right @pawelfus? If you agree, your team can work on a fix for it.

@romanukas

This comment has been minimized.

romanukas commented Aug 31, 2017

Completely agree with @juicelink : YTD range should have start point at 31 dec (with 0 performance). I work in the finance sector and that's the truth. When do you expect to fix this?

@juicelink

This comment has been minimized.

juicelink commented Sep 13, 2017

Did you team take the issue?
If not can try to make a pr. Just not want to work on it if you do.

@pawelfus pawelfus added this to the v6.0.0 milestone Sep 13, 2017

@pawelfus

This comment has been minimized.

Contributor

pawelfus commented Sep 13, 2017

It will be resolved in the next major version.

pawelfus added a commit that referenced this issue Sep 18, 2017

pawelfus added a commit that referenced this issue Sep 19, 2017

@Harvzor

This comment has been minimized.

Harvzor commented Oct 5, 2017

Hi guys, thanks for the great work here. Just in time to fix some issues I've been having in my project!

However, the example for compareStart in the documentation doesn't actually seem to use compareStart. It seems to use a slightly hacky fix for YTD where it adjusts the time span using offsetMin.

Also, the documentation says that the default for compareStart is undefined, when I think the default is actually false. It doesn't specifically say that setting it to true will change the base to be calculated as 0%, rather than from the previous point, but from my tests this appears to be the case.

I have tested the YTD fix and the compareStart feature and they both fix the bugs which I was having. Thanks again!

@pawelfus

This comment has been minimized.

Contributor

pawelfus commented Oct 5, 2017

Hi @HarveyWilliams

Thank you for the info!

@pawelfus pawelfus reopened this Oct 5, 2017

@juicelink

This comment has been minimized.

juicelink commented Oct 8, 2017

tried the compareStart option on the demopage. Seems that the behavior isn't consistent. When using the horizontal scroll bar to select period then the first point isn't the first day of the period
hc

(after verification same inconsistence with compareStart at false : if you set range with date fields or button the first point of the range is shown; if you change the period with the scrollbar the first point is hidden)

@pawelfus

This comment has been minimized.

Contributor

pawelfus commented Oct 9, 2017

Hi @juicelink

Could tell me exact dates when it fails? Working demo: http://jsfiddle.net/8pktsngx/

@juicelink

This comment has been minimized.

juicelink commented Oct 9, 2017

Sorry but not sure to understand the question. At any date. If you change the period by Moving the Scroll bar, first point is one day later than start date shown in range fields. (Same issue in jsfiddle) (in previous picture, first point at june 20, 0%, in the range field start date at june 19)

@pawelfus

This comment has been minimized.

Contributor

pawelfus commented Oct 9, 2017

Thanks for the explanation! "date shown in range fields" was the hint here. Do you mean that rangeSelector inputs are wrong? I'm not sure if this is a bug, let's see what really happens: http://jsfiddle.net/8pktsngx/1/ - by panning you can get for example:
screen shot 2017-10-09 at 18 06 55

As you can see, first point is 18th of Sep, but rangeSelector shows 17th of Sep. Points in time are exactly at 00:00:00.000 (UTC), so actually this is correct, point 17th of Sep is out of the given range.

@juicelink

This comment has been minimized.

juicelink commented Oct 9, 2017

It makes sense. Thank's for the explanation!

@juicelink

This comment has been minimized.

juicelink commented Oct 13, 2017

I´m not sure that offsetMin really fixes the YTD issue as explained in #687

that we want to calculate financial YTD is to include the last point of the previous year. More generally when we calculate a performance we want to include the point predecessing the period. But sometimes points are not at same distance (week ends, trading hours...). So it seems useless to extend the period in term of milliseconds as for example for YTD, depending of the year we don´t know if we must add 1, 2 or 3 days.
A similar option that adds a number of points instead of adding milliseconds should do the trick

To resume YTD for 6.0 version:

  • A : with compareStart = false, offsetmin = 0
    YTD value seems correct. last point of previous year included in period. But this point doesn´t appear on the chart. Range selector starts at 01/01, First point percentage doesn´t start at 0
  • B : with comparestart = true, offsetmin = 1day
    YTD is false. It starts at first value of january as 31 dec is a weekend day. First point percentage point starts at 0. start date in range selector is same as the chart´s first point date.

What I would expect:
YTD calculated from Friday dec 30 to now (as in A)
Friday dec 30 in start date field of the range selector
first point of the chart at Friday dec 30 with 0%

@romanukas

This comment has been minimized.

romanukas commented Sep 5, 2018

@juicelink do I understand correctly that this is not just YTD, but universal problem - for any range selected? If range start date does not have a price, then actually chart should show last known price before it, not after it?

@romanukas

This comment has been minimized.

romanukas commented Sep 5, 2018

@pawelfus is anybody working on this?

@pawelfus

This comment has been minimized.

Contributor

pawelfus commented Sep 5, 2018

@romanukas This feature is actually implemented - it's possible to control YTD button behaviour (and all other buttons too!). Issue was reopened because of bug in docs and demo only (which seems to be fixed too).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment