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

Possible regression because of recent startAtZeroEnabled changes #300

Closed
yas375 opened this issue Aug 13, 2015 · 11 comments
Closed

Possible regression because of recent startAtZeroEnabled changes #300

yas375 opened this issue Aug 13, 2015 · 11 comments

Comments

@yas375
Copy link
Contributor

yas375 commented Aug 13, 2015

Without any changes to my code here is what I get if I use this library from 2177f7d

2015-08-13_1639

And here is what I'm getting from current master (2068a5c):

2015-08-13_1633
(Note -15 in the left axis and extra space)

I guess it might be related to recent changes around startAtZeroEnabled. In my code I don't this property.

Here are some chart params I set which might be useful to know for debugging the issue:

self.chartView.drawGridBackgroundEnabled = NO;
self.chartView.doubleTapToZoomEnabled = NO;
self.chartView.pinchZoomEnabled = YES;
self.chartView.scaleYEnabled = NO;

self.chartView.legend.form = ChartLegendFormCircle;

self.chartView.xAxis.drawGridLinesEnabled = NO;
self.chartView.xAxis.avoidFirstLastClippingEnabled = YES;
self.chartView.xAxis.labelPosition = XAxisLabelPositionBottom;

self.chartView.rightAxis.enabled = NO;
ChartDataEntry *dataEntry = [[ChartDataEntry alloc] initWithValue:value xIndex:index];
LineChartDataSet *dataSet = [[LineChartDataSet alloc] initWithYVals:values label:@"bar"];

dataSet.lineWidth = 2.0;
dataSet.drawFilledEnabled = YES;
dataSet.circleRadius = 4.0;
dataSet.drawValuesEnabled = NO;
dataSet.drawHighlightIndicators = NO;
@danielgindi
Copy link
Collaborator

startAtZeroEnabled is meant for positive values, when you want to see the 0 point in the axis. It normalizes the axis.
But when you have negative values, we can't just ignore them, they need to be visible.
The new behaviour is the correct one. If you don't want negative values to be visible you can either remove them, or set a custom y axis minimum to 0.

@liuxuan30
Copy link
Member

I am curious, @yas375 your data only has positive values (first screenshot), but it calculated a negative _yAxisMinimum in second screenshot?
@danielgindi I think the problem is, the values are positive, but we calculated something negative.

@danielgindi
Copy link
Collaborator

Well I assumed he has -15 somewhere in his dataset, that is out of the visible range...

@liuxuan30
Copy link
Member

Yes, in the screenshot I don't see any negative. Need @yas375 to double check.

@yas375
Copy link
Contributor Author

yas375 commented Aug 14, 2015

Thanks for the quick feedback!

I don't have any negative data entries:

ChartDataSet, label: Bar, 27 entries:
ChartDataEntry, xIndex: 0, value 103.0
ChartDataEntry, xIndex: 1, value 134.0
ChartDataEntry, xIndex: 3, value 202.0
ChartDataEntry, xIndex: 4, value 237.0
ChartDataEntry, xIndex: 7, value 131.0
ChartDataEntry, xIndex: 9, value 114.0
ChartDataEntry, xIndex: 10, value 238.0
ChartDataEntry, xIndex: 11, value 184.0
ChartDataEntry, xIndex: 12, value 274.0
ChartDataEntry, xIndex: 13, value 149.0
ChartDataEntry, xIndex: 14, value 203.0
ChartDataEntry, xIndex: 17, value 134.0
ChartDataEntry, xIndex: 18, value 81.0
ChartDataEntry, xIndex: 19, value 253.0
ChartDataEntry, xIndex: 20, value 250.0
ChartDataEntry, xIndex: 51, value 230.0
ChartDataEntry, xIndex: 52, value 214.0
ChartDataEntry, xIndex: 54, value 116.0
ChartDataEntry, xIndex: 55, value 260.0
ChartDataEntry, xIndex: 56, value 163.0
ChartDataEntry, xIndex: 57, value 234.0
ChartDataEntry, xIndex: 58, value 91.0
ChartDataEntry, xIndex: 59, value 207.0
ChartDataEntry, xIndex: 60, value 92.0
ChartDataEntry, xIndex: 64, value 92.0
ChartDataEntry, xIndex: 65, value 132.0
ChartDataEntry, xIndex: 76, value 12.0

And I have 77 values for xAxis and I believe you can use any strings there.
Let me know guys if you can't reproduce the issue with this data and I can try to make a sample project.

Thanks again!

@yas375
Copy link
Contributor Author

yas375 commented Aug 14, 2015

Also I've verified locally that 67cb278 makes the difference. Without this commit it works as it was before (my first screenshot).

@danielgindi
Copy link
Collaborator

Then it must mean that in your case, for some reason, axisMinimum is less than zero when the decision to not start at zero is made in the code. Somewhere there in 67cb278... Do you mind debugging it a little? Or giving a small project demonstrating the problem?

@danielgindi danielgindi reopened this Aug 15, 2015
@alinoz77
Copy link

The -15 value comes form the chart bottomSpaceLeft.
Without the 67cb278 change the _leftAxis.axisMinimum was set to zero even if you didn't set the leftAxis.customAxisMin.
Now with this change the value is not set to zero and the default is going to use the value calculated in the BarLineChartViewBase.swift at line 324:

        _leftAxis.axisMinimum = !isnan(_leftAxis.customAxisMin) ? _leftAxis.customAxisMin : (minLeft - bottomSpaceLeft)

Don't know the complete library to well so I can not decide if the bottomSpaceLeft is really needed, but maybe Daniel can have a look and decide.

@danielgindi
Copy link
Collaborator

Oh I got it. It's something I missed. bottom spacing is not needed when
starting at zero

‏בתאריך יום שני, 17 באוגוסט 2015, alinoz77 notifications@github.com כתב:

The -15 value comes form the chart bottomSpaceLeft.
Without the 67cb278
67cb278
change the _leftAxis.axisMinimum was set to zero even if you didn't set the
leftAxis.customAxisMin.
Now with this change the value is not set to zero and the default is going
to use the value calculated in the BarLineChartViewBase.swift at line 324:

    _leftAxis.axisMinimum = !isnan(_leftAxis.customAxisMin) ? _leftAxis.customAxisMin : (minLeft - bottomSpaceLeft)

Don't know the complete library to well so I can not decide if the
bottomSpaceLeft is really needed, but maybe Daniel can have a look and
decide.


Reply to this email directly or view it on GitHub
#300 (comment)
.

@danielgindi
Copy link
Collaborator

Will fix it soon

‏בתאריך יום שני, 17 באוגוסט 2015, Daniel Cohen Gindi danielgindi@gmail.com
כתב:

Oh I got it. It's something I missed. bottom spacing is not needed when
starting at zero

‏בתאריך יום שני, 17 באוגוסט 2015, alinoz77 <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> כתב:

The -15 value comes form the chart bottomSpaceLeft.
Without the 67cb278
67cb278
change the _leftAxis.axisMinimum was set to zero even if you didn't set the
leftAxis.customAxisMin.
Now with this change the value is not set to zero and the default is
going to use the value calculated in the BarLineChartViewBase.swift at line
324:

    _leftAxis.axisMinimum = !isnan(_leftAxis.customAxisMin) ? _leftAxis.customAxisMin : (minLeft - bottomSpaceLeft)

Don't know the complete library to well so I can not decide if the
bottomSpaceLeft is really needed, but maybe Daniel can have a look and
decide.


Reply to this email directly or view it on GitHub
#300 (comment)
.

@yas375
Copy link
Contributor Author

yas375 commented Aug 17, 2015

Thanks a lot!

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

No branches or pull requests

4 participants