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 radar chart negative value rendering bug if startAtZeroEnabled is false for issue #166 #207

Merged
merged 1 commit into from
Aug 12, 2015

Conversation

liuxuan30
Copy link
Member

For issue #166
we have to check if Double(yMin) / interval will be -1<x<0, if this is the case, ceil(x) would be 0. but I think we need it to be -1.

negative values using ceil is not correct, I think. This will lead the minimum value is larger than the data values, like if I have -0.5 , 1 , it calculate as [0,1], not [-1,1]. (ceil(-0.5) = 0)

In order to render the negative values correctly,

_yAxis.axisMinimum = _yAxis.entries[0];

is the key.

I have been thinking about different ways to fix. But I was suddenly realized the whole reason is the axisMinimum is not changed while the entries have been modified. Add this line solved my problem, and other radar charts remain good on my side. So I think this should be the correct way to fix it.

@danielgindi you must double check here, because I am not sure if you missed to set it, or you intend not to set _yAxis.axisMinimum.

@danielgindi
Copy link
Collaborator

I don't see this problem occurring in latest versions am I right?
Since startAtZero behaviour was fixed when working on the negative stacked bars...
About axisMinimum, it is set in calcMinMax

@liuxuan30
Copy link
Member Author

I will try when I got a chance. working on huge svg-style maps recently and it drives me crazy

@liuxuan30
Copy link
Member Author

@danielgindi I still don't think it is fixed. Paste dataSet code at bottom for you to test.

There are two parts:

  1. I think the logic to compute var first is not correct for negative value. You use ceil for it, but I think we should use floor. Consider ceil(-99.5) and floor(-99.5). This issue is generic for other charts, as this is quite old opened issue, I want you to review it and apply to all charts.
  2. Yes _yAxis.axisMinimum is calculated in calcMinMax, but somehow I found I have to assign it again after we get new _yAxis.entries for radar chart. Radar chart is the first chart I started, so at that time, I don't know many details for all the charts, but want to fix the bug.

v2.1.3 radar chart without my code:
radarbug

radar chart with my code:
radarfix

the distance between the center point to first web and first web to second web is not identical in master branch code.

dataSet code to test:

- (void)setData
{
    double mult = 150.f;
    int count = 5;

    NSMutableArray *yVals1 = [[NSMutableArray alloc] init];
    NSMutableArray *yVals2 = [[NSMutableArray alloc] init];
    NSMutableArray *yVals3 = [[NSMutableArray alloc] init];

    [yVals1 addObject:[[ChartDataEntry alloc] initWithValue:-7501.574034 xIndex:0]];
    [yVals2 addObject:[[ChartDataEntry alloc] initWithValue:35790.059533 xIndex:0]];
    [yVals3 addObject:[[ChartDataEntry alloc] initWithValue:42567.845168 xIndex:0]];

    [yVals1 addObject:[[ChartDataEntry alloc] initWithValue:73587.954072 xIndex:1]];
    [yVals2 addObject:[[ChartDataEntry alloc] initWithValue:20118.91748 xIndex:1]];
    [yVals3 addObject:[[ChartDataEntry alloc] initWithValue:187068.443587 xIndex:1]];

    [yVals1 addObject:[[ChartDataEntry alloc] initWithValue:68861.089668 xIndex:2]];
    [yVals2 addObject:[[ChartDataEntry alloc] initWithValue:42567.845168 xIndex:2]];
    [yVals3 addObject:[[ChartDataEntry alloc] initWithValue:277395.375557 xIndex:2]];

    [yVals1 addObject:[[ChartDataEntry alloc] initWithValue:79499.134258 xIndex:3]];
    [yVals2 addObject:[[ChartDataEntry alloc] initWithValue:38749.916356 xIndex:3]];
    [yVals3 addObject:[[ChartDataEntry alloc] initWithValue:133373.871463 xIndex:3]];

    [yVals1 addObject:[[ChartDataEntry alloc] initWithValue:53615.304423 xIndex:4]];
    [yVals2 addObject:[[ChartDataEntry alloc] initWithValue:13551.626137 xIndex:4]];
    [yVals3 addObject:[[ChartDataEntry alloc] initWithValue:132363.776504 xIndex:4]];

    NSMutableArray *xVals = [[NSMutableArray alloc] init];

    for (int i = 0; i < count; i++)
    {
        [xVals addObject:parties[i % parties.count]];
    }

    RadarChartDataSet *set1 = [[RadarChartDataSet alloc] initWithYVals:yVals1 label:@"Set 1"];
    [set1 setColor:ChartColorTemplates.vordiplom[0]];
    set1.drawFilledEnabled = YES;
    set1.lineWidth = 2.0;

    RadarChartDataSet *set2 = [[RadarChartDataSet alloc] initWithYVals:yVals2 label:@"Set 2"];
    [set2 setColor:ChartColorTemplates.vordiplom[4]];
    set2.drawFilledEnabled = YES;
    set2.lineWidth = 2.0;

    RadarChartDataSet *set3 = [[RadarChartDataSet alloc] initWithYVals:yVals3 label:@"Set 3"];
    [set3 setColor:ChartColorTemplates.vordiplom[4]];
    set3.drawFilledEnabled = YES;
    set3.lineWidth = 2.0;

    RadarChartData *data = [[RadarChartData alloc] initWithXVals:xVals dataSets:@[set1, set2, set3]];
    [data setValueFont:[UIFont fontWithName:@"HelveticaNeue-Light" size:8.f]];
    [data setDrawValues:NO];

    _chartView.yAxis.startAtZeroEnabled = NO;
    _chartView.data = data;
}

@danielgindi
Copy link
Collaborator

Well I get the floor/ceil thing, but the axisMinimum seems wrong to me.

As the yAxis range - what happens is that the Radar chart override computeAxisValues to calculate a slightly different set of values to show on the yAxis. But after normalizing the y axis, the labels below are showing on the wrong side of the axis.

Now we can do one of two things:

  1. Adjust the axisMinimum like you do, to make the whole range show above the center. This has the side-effect the the minimum value is not at the exact center of the chart.
  2. Crop, make the labels that are below the range just not show.

In my opinion the first value should be at the center am I not right?

@danielgindi
Copy link
Collaborator

Btw, do not forget about the "spaceBottom" which adds spacing to the yAxis at the bottom

@danielgindi danielgindi merged commit f76aae9 into ChartsOrg:master Aug 12, 2015
@danielgindi
Copy link
Collaborator

Okay I've merged with some changes to handle startAtZero correctly (This affects bar charts also)

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.

None yet

2 participants