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

NaN point value makes series disappear #3571

Closed
albhilazo opened this Issue Nov 7, 2014 · 13 comments

Comments

Projects
None yet
8 participants
@albhilazo

albhilazo commented Nov 7, 2014

Didn't find any report on this issue so here it goes.
http://jsfiddle.net/993ee7cx/
If an area or line series has a point with 'NaN' value, the whole line disappears. In the previous example you can see the points, but for series with hundreds of points you can only see the tooltip.
http://jsfiddle.net/993ee7cx/1/

Null values seem to be controlled, but since Error #14 recommends the use of 'parseFloat()' and it returns NaN for null values it can be a problem for DB retrieved data.

For the time being I went around it with a simple check

seriesdata.push({
    'data': [ ( (isNaN(parseFloat(stringval))) ? null : parseFloat(stringval) ) ]
});
@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Nov 7, 2014

Collaborator

Not sure how to handle this... Our docs do say that values should be either numbers or null. Checking for isNaN would add another case to check for and decrease performance.

Collaborator

TorsteinHonsi commented Nov 7, 2014

Not sure how to handle this... Our docs do say that values should be either numbers or null. Checking for isNaN would add another case to check for and decrease performance.

@albhilazo

This comment has been minimized.

Show comment
Hide comment
@albhilazo

albhilazo Nov 7, 2014

I understand. Just thought it was worth mentioning, since it took me a while to find the reason why the series weren't being drawn without error.
Amazing library anyway, thanks!

albhilazo commented Nov 7, 2014

I understand. Just thought it was worth mentioning, since it took me a while to find the reason why the series weren't being drawn without error.
Amazing library anyway, thanks!

@filmor

This comment has been minimized.

Show comment
Hide comment
@filmor

filmor Apr 14, 2016

It makes a lot of sense to add this check. We are using highcharts with typed arrays which can contain NaNs. The current buggy handling of NaN values leads to an additional conversion into a js array which costs us far more performance than this simple check would. Maybe you could make it configurable?

filmor commented Apr 14, 2016

It makes a lot of sense to add this check. We are using highcharts with typed arrays which can contain NaNs. The current buggy handling of NaN values leads to an additional conversion into a js array which costs us far more performance than this simple check would. Maybe you could make it configurable?

@epa

This comment has been minimized.

Show comment
Hide comment
@epa

epa Jul 12, 2016

Could I add a 'me too' to this. The bug is that if NaN appears in the data then the series is drawn in invisible ink. You can still hover over it with the mouse but the line isn't shown. This really doesn't make any sense; by all means fail if NaNs are given but not in this weird way which isn't immediately visible. (If there are ten series on the chart and one of them becomes invisible without warning...)

epa commented Jul 12, 2016

Could I add a 'me too' to this. The bug is that if NaN appears in the data then the series is drawn in invisible ink. You can still hover over it with the mouse but the line isn't shown. This really doesn't make any sense; by all means fail if NaNs are given but not in this weird way which isn't immediately visible. (If there are ten series on the chart and one of them becomes invisible without warning...)

@epa

This comment has been minimized.

Show comment
Hide comment
@epa

epa Jul 22, 2016

Thanks!

epa commented Jul 22, 2016

Thanks!

@albhilazo

This comment has been minimized.

Show comment
Hide comment
@albhilazo

albhilazo commented Jul 22, 2016

Thanks!

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jul 25, 2016

Collaborator

You're welcome!

Collaborator

TorsteinHonsi commented Jul 25, 2016

You're welcome!

@cnrudd

This comment has been minimized.

Show comment
Hide comment
@cnrudd

cnrudd Aug 25, 2016

Thanks for fixing. I've tested the fix.

There is one edge case that does not appear to be fixed: a NaN at the very first point of a timeseries will still cause the whole timeseries to disappear.

Not a big deal for us, but it seems like it might be worth mentioning.

cnrudd commented Aug 25, 2016

Thanks for fixing. I've tested the fix.

There is one edge case that does not appear to be fixed: a NaN at the very first point of a timeseries will still cause the whole timeseries to disappear.

Not a big deal for us, but it seems like it might be worth mentioning.

@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Aug 26, 2016

Contributor

Thank you for pointing this out! Live demo: http://jsfiddle.net/993ee7cx/2/

Internal note:
Extremes for series are NaN in this case, here we should also use isNumber() instead of comparing with null and undefined.

Contributor

pawelfus commented Aug 26, 2016

Thank you for pointing this out! Live demo: http://jsfiddle.net/993ee7cx/2/

Internal note:
Extremes for series are NaN in this case, here we should also use isNumber() instead of comparing with null and undefined.

@pawelfus pawelfus reopened this Aug 26, 2016

TorsteinHonsi added a commit that referenced this issue Aug 29, 2016

@grovesNL

This comment has been minimized.

Show comment
Hide comment
@grovesNL

grovesNL Nov 7, 2017

@TorsteinHonsi Similarly to NaN, what's the expected behavior for Infinity and -Infinity? I can't find anything in the documentation about it, and it seems relevant to this issue. I can open a new issue if you'd like.

grovesNL commented Nov 7, 2017

@TorsteinHonsi Similarly to NaN, what's the expected behavior for Infinity and -Infinity? I can't find anything in the documentation about it, and it seems relevant to this issue. I can open a new issue if you'd like.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Nov 8, 2017

Collaborator

Interesting question... Infinity and -Infinity are numbers, but we can't plot the on the Y axis, so maybe they should be treated as invalid values. It is quite simple to work around that by modifying the Highcharts.isNumber function:

H.isNumber = function (n) {
	return typeof n === 'number' && !isNaN(n) && n !== Infinity && n !== -Infinity;
};

On the other hand, Number.MAX_VALUE is actually a number that can be plotted, but currently Highcharts chokes on it and displays an empty chart.

Collaborator

TorsteinHonsi commented Nov 8, 2017

Interesting question... Infinity and -Infinity are numbers, but we can't plot the on the Y axis, so maybe they should be treated as invalid values. It is quite simple to work around that by modifying the Highcharts.isNumber function:

H.isNumber = function (n) {
	return typeof n === 'number' && !isNaN(n) && n !== Infinity && n !== -Infinity;
};

On the other hand, Number.MAX_VALUE is actually a number that can be plotted, but currently Highcharts chokes on it and displays an empty chart.

@grovesNL

This comment has been minimized.

Show comment
Hide comment
@grovesNL

grovesNL Nov 8, 2017

Yeah, I'm not sure what users would expect to happen in these cases either. I guess treating them as invalid and filtering out the points is the most straightforward solution (there is also Number.isFinite in some browsers).

grovesNL commented Nov 8, 2017

Yeah, I'm not sure what users would expect to happen in these cases either. I guess treating them as invalid and filtering out the points is the most straightforward solution (there is also Number.isFinite in some browsers).

@sebastianbochan

This comment has been minimized.

Show comment
Hide comment
@sebastianbochan

sebastianbochan Jun 18, 2018

Contributor

@TorsteinHonsi maybe we should also add the support for NaN as string?

Contributor

sebastianbochan commented Jun 18, 2018

@TorsteinHonsi maybe we should also add the support for NaN as string?

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