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

Fix decimals crash in ChartsUtil #1558

Merged
merged 2 commits into from
Sep 29, 2016
Merged

Fix decimals crash in ChartsUtil #1558

merged 2 commits into from
Sep 29, 2016

Conversation

pmairoldi
Copy link
Collaborator

@pmairoldi pmairoldi commented Sep 25, 2016

Closes #1550, #1540, #1491

I added some tests based on the crash mentioned in #1550 and there was a crash when using DBL_MAX as the input of the decimals function.

There are still some abnormalities in the decimals function I believe. In MPAndroidCharts I ran the same tests and it returns 2 not zero for all these cases except for the ones that crash and return negative Int max.

This pull request fixes the crash but I don't think the decimals function does what we want it to do.

@danielgindi, @PhilJay what is this function actually supposed to do?

@danielgindi
Copy link
Collaborator

It's supposed to calculate a default number of decimals to show after the decimal point

@danielgindi
Copy link
Collaborator

It's not really important what the value is on edge cases. It is used as a default for formatters that were not configured by the user, it just shouldn't crash :-)

@danielgindi danielgindi merged commit 7bc798b into master Sep 29, 2016
@danielgindi danielgindi deleted the fix-decimals-crash branch September 29, 2016 05:11
@pmairoldi
Copy link
Collaborator Author

The main reason I was asking what it does is that I believe it does not do what it's supposed to. See the testDecimalWithNormalValue test case. Shouldn't it return 6? If not then why are we doing some complex math if it can only be 0 or 1.

@umairnow
Copy link

umairnow commented Dec 5, 2016

This issue is still there in AxisRendererBase.swift on line 125
fatal error: Double value cannot be converted to Int because it is either infinite or NaN

@acegreen
Copy link

acegreen commented Mar 10, 2017

@petester42 @danielgindi this isn't resolved yet..

        let yRangeBuffer: Double = 50000
        lineChartView.xAxis.axisMinimum = set1.yMin - yRangeBuffer
        lineChartView.xAxis.axisMaximum = set1.yMax + yRangeBuffer

also crash with something as simple as:

lineChartView.xAxis.spaceMin = yRangeBuffer
lineChartView.xAxis.spaceMax = yRangeBuffer

@danielgindi
Copy link
Collaborator

Guys, the reason that this is not fixed, is that we do not see this error.
So you would have to:

  1. Make sure you are doing everything correctly
  2. Create steps for us to reproduce
  3. Maybe do a little debugging. Why does a NaN or Infinity get passed to that function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fatal error: Double value cannot be converted to Int because it is either infinite or NaN
4 participants