Conforms to Swift 3 - Xcode 8 beta 6

[acegreen]

No description provided.

[acegreen]

@liuxuan30 there could be a few errors in this. I would go over with a fine-tooth comb. I noticed my Pie chart hole is no longer transparent. Looking into that

[liuxuan30]

are you saying there are issues with the library or xcode beta?

[acegreen]

@liuxuan30 No the changes introduced because of beta 6 might have caused some small errors. I noticed my pie chart hole is no longer transparent for example. Not sure if this is related to the changes or from before. All I'm saying is we should go through this before merging it.

PS By errors I don't mean it wont compile. There could be some copy/paste mistakes

[liuxuan30]

well.. I assume the conversion tool will take care of all of them. Why you said copy/paste mistakes? What and where needs you manual copy? :) Downloading beta 6...

[acegreen]

I do most of my changes manually to make sure its the intended change and for example
context.move(to:) now takes a CGPoint, so in some cases, I MIGHT have not gotten right. Again I'm being overly cautious with my statements. Could also be that everything is 100%. So again to summarize, I didn't catch any mistakes in my review but there could be.

[acegreen]

@liuxuan30 this is a mistake. Surprising since no change needed to be made on that line

[liuxuan30]

such changes should be included I guess, if not converted by Xcode 8. I would only use the conversion tool to do the job, since it's all about swift 3.0, not about bugs or issues. If you manually changed something, with so many diff in one PR, it's hard to find out.

Anyway, swift 3.0 branch is just for testing

[acegreen]

Not sure what you are saying. That change is simply a type, I removed count: by accident and put it back and instead of 4, I put it back as 2. For other changes, I don't use the tool because with Swift 3 it causes syntax issues especially in this beta. Also many changes in this beta were not suggested.

[liuxuan30]

hmm, it seems get complicated... I mean the conversion tool will not change count 4 to 2 right? It seems like a manual change. I am asking why we change from 4 to 2.
Also, why you said "many changes in this beta were not suggested" ? I am still downloading, but I need to work then.

[acegreen]

Yea give the conversion tool a try this time around, then you will understand. Anyways in this instance its simply a mistake. I can submit another commit with the fix once we go over the rest, to make sure there aren't any other mistakes.

[liuxuan30]

@acegreen I now understand. However, I think _shadowPoints count should be 4. not 2. Code below uses 4 of them.

[acegreen]

haha ITS MY MISTAKE :P, I removed the count and when I added it, I added it back as 2. IT SHOULD BE 4. I'll update it in the next commit

[liuxuan30]

@acegreen hey I just spend several hours on this. Thanks for your work, I know it's painful and requires patience.

There are several issues we need to address (also in line note):

1. public init(value: Double, xIndex: Int, data: Any?) in ChartDataEntry
2. eoFillPath(), also not sure if this is related to your pie chart issue
3. string(for:)
4. private var _shadowPoints = [CGPoint](repeating: CGPoint%28%29, count: 2)

You could continue commit in this PR
I am on vacation this week, so might be late in response

[pixelspark]

Great work @acegreen! Unfortunately some build issues when building for OS X as well (many of which are simple fix its, it appears). Might have time to submit a PR later this week (if you haven't applied the fix-its already yourself by then :))

Edit: was easier than expected, see #1353

[acegreen]

@liuxuan30 Yea but we should be able to sort it out.

1. Can you elaborate on this?
2. I wrongly assumed fillPath() is equivalent to eoFillPath() but it should be
   fillPath(using: .evenOdd). I changed 4 instances back to that.
3. Reverted back to string(from: ) but casted values as NSNumber
4. Corrected back to count: 4

[liuxuan30]

@acegreen I am going to merge this PR soon, but a few more comments:

for 1, I am just saying using Any and later self.data = data as AnyObject seems weird.. but you give me SE0116, so let's move forward.

for 2, have you investigated .evenOdd or winding, and relates to your pie chart issue?
for 3, my beta 6 told me to use formatter.string(from: NSNumber(val)) instead of as, though they look equivelant, so I am fine.

[acegreen]

1. Making data: Any in init. seems right an der Casting it to AnyObject is sometimes required if we need a specific feature only available to AnyObject, which in these case we do. Apple mentions this in the release notes of Xcode 8 beta 6.
2. I investigated that and it turns out there is an .evenOdd option in fillPath. Which is what i should have used in the first place for all eoFillPath() (eo = evenOdd) but instead I used just fillPath(). Second commit reverts back to using evenOdd with the Swift 3 syntax
3. Both are the same, though Apple recommends casting "as" for dealing with Swift to objc equivalent. I think it's SE-0072
   Using .evenOdd addresses the pie chart issue and is back to the way it was (which is good)

[liuxuan30]

great, merged. Thanks!