stsievert / swix

Swift Matrix Library
http://docs.stsievert.com/swix/
MIT License
593 stars 54 forks source link

Convert to Swift3 & make it a framework #28

Closed thomaskilian closed 7 years ago

thomaskilian commented 8 years ago

Hi there, I have converted the whole stuff to Swift3 (can't wait for 3.1, 3.2, 4, etc) and made changes to create a framework.

thomaskilian commented 8 years ago

I just noticed that though the framework compiles ok, there are issues when trying to use it. I have to dig into that first.

thomaskilian commented 8 years ago

Fail. I'll still have to do more homework. Will come back later...

stsievert commented 8 years ago

Thanks for the work on this! I've been meaning to fix this for a while, as mentioned in #23. I'll probably use your branch when I finally get around to this.

stsievert commented 8 years ago

I couldn't get your version to run with Xcode 8. Looks like we should build Swift.framework then have a separate example that includes that Swift.framework. That'll make the install much easier.

thomaskilian commented 8 years ago

I was a bit shocked when I found out how difficult it gets once you want to create a framework with mixed obj-c/swift code (despite Apple's saying). I'm currently experimenting with that and got some parts to work. Now I'm looking into modulemap. What I could do is to create a new fork with just the Swift3-conversion in a first step.

thomaskilian commented 8 years ago

Phew - I got one step further and can compile the FW but not fully use it. It appears that exposed Swift sources need to be on top level and are hidden when inside groups. WTF? (There is also a -Swift.h containing the needed info which could be manipulated, but...) I found someone else having the same issue and the "solution" seems to be to create two projects. One for creating the FW and one for source control with appropriate groups. Someone should club those guys at Apple to death with a tea spoon. Preferably one made of plastic so it takes longer.

thomaskilian commented 8 years ago

Hu, found it! You have to add lots of public keywords. I did so at a few places, but would like to avoid to do that for everything since some parts are likely private.

stsievert commented 8 years ago

Are you completely pushed? I've tried opening the xcodeproj on your fork but find Swix.framework, can't build, etc.

I'm fine adding public keywords if the functions are meant to be public (only unused function should be private).

Ideally I'd like separate PRs for Swift 3 and making a framework.

thomaskilian commented 8 years ago

Hmm, I added a few more public statements meanwhile, but it should already work. Maybe I should first re-create the Swift3 change (to be put on a new branch, I guess)?

thomaskilian commented 8 years ago

What kind of error did you encounter?

thomaskilian commented 8 years ago

I have downloaded my latest commit as zip and compiled with no issues. I'm using XCode Version 8.0 (8A218a)

thomaskilian commented 8 years ago

I just added a new branch Swift3. The first commit is wishful thinking xD This should compile just with Swift3 and also has a minor change in the framework search paths (just searching swix recursively and leave out the wrong ones)

thomaskilian commented 8 years ago

Just stumbled over this one: http://stackoverflow.com/questions/27049129/swift-how-to-create-custom-operators-to-use-in-other-modules I started a bounty, since this would make the framework a patchwork :-(

stsievert commented 8 years ago

Could you start a new PR with Swift3? I got it to compile and most tests pass (but not the file io tests).

I'm running the same version of Xcode, 8A218a. The swift3 branch compiled and ran, the master branch didn't.

thomaskilian commented 8 years ago

What are the errors you get with the master branch?

thomaskilian commented 8 years ago

Just one question: is there a good reason for not putting the functions like min, max etc inside the vector/matrix structs? So you rather would call someVector.min iso min(someVector). That would also not flood the name space. They could as well be defined as extensions.

I guess the biggest drawback (besides the tense Python syntax compared to Swift) is the fact that you can not easily create matrices from vector-arrays.

stsievert commented 8 years ago

is there a good reason for not putting the functions like min, max etc inside the vector/matrix structs

I was trying to minic pylab which includes max(x) and x.max().

I guess the biggest drawback (besides the tense Python syntax compared to Swift) is the fact that you can not easily create matrices from vector-arrays.

Perfect, I've been waiting hear this kind of feedback. This should only be a small pain (make the appropriate sized matrix then fill in the columns). I'd welcome a PR -- this project always seems to fall by the wayside for me :/

thomaskilian commented 8 years ago

There are a couple of issues I'd like to work on, but I'll try keeping an eye on this. So this is what I see as next steps:

What do you think? I have quite some time to dive into this.

stsievert commented 8 years ago

Perfect! I'd like to see a separate PR for each of these.

get the framework to run also for you (why does it fail??)

I ran into an issue where I couldn't even build the .xcodeproj. Swix.framework wasn't found, etc.

Try to mimic the array of vectors to be treated as matrix just as NumPy can do (I need to think about this)

I think providing a conversion utility between an array of vectors and a matrix would be ideal. This can support OOP.

split between a basically OO framework (vector.max etc.) and an optional functional framework (max(vector) etc.).

What about sin, exp, etc? I'd rather see something similar to Python's swix.sin. I'm not familiar with how Swift package management works but I'd like to go with the mainstream solution (which I believe is pods).

thomaskilian commented 8 years ago

Ok. So let's eat the elephant in slices. Firstly we need to sort out this issue with building the project. I thought that a project is self-containing and should compile the same way everywhere. Can you share the error log from the compilation?

thomaskilian commented 7 years ago

What OSX version do you have installed? Is it Sierra? I changed the deployment target to 10.12 (which is not necessary, but was easier in the moment for other circumstances). If that's the reason, I try to set the deployment to an earlier base.

thomaskilian commented 7 years ago

I did set the deployment back to 10.9

thomaskilian commented 7 years ago

I have bundled the tests in a single app. This is because command line tools can not easily integrate frameworks while apps can. Since this is for a test only, I think it's ok. Further I had to add a lot more brackets because Swift3 handles operators more strict (which is good and bad: e.g. a*b/c now needs explicit brackets to order division and multiplication which usually is crucial). I will have to look into these operators in more detail.

There are now two targets: one for the test and one for the framework itself. Once built, it can be placed in /Library/Frameworks so it can be used by other apps.

I temporarily gave up on splitting OO and functional access for vector/matrix since I ran into an issue I could not solve instantly. I'll have to make further tests first.

thomaskilian commented 7 years ago

As a side note: I have named the framework SwixOO because I wanted to have a second SwiftF for the functional access. As said, this currently fails. However, a framework must have a different name than the project itself (WTF?). Either I get this OO/functional thingy to work the next days or there needs to be some name change of either the project or the framework (I loathe to name it SwiftFramework).

thomaskilian commented 7 years ago

Another observation: Classes and Structures shall have an uppercase first letter. Now, you are using vector and matrix so they need to be rather Vector and Matrix. Would that be a problem for you?

thomaskilian commented 7 years ago

Another issue: the copy function you provide for vector clashed with the copy function which is provided in an AppDelegate. Could this be renamed to copyVector?

stsievert commented 7 years ago

I'd rather not rename it to copyVector -- copy is appropriate for this library. We should probably move it to vector.copy to avoid this conflict.

I'd like this PR to only be on converting to Swift 3. Once all the tests (including the IO tests) and all other functionality is removed, I'll merge it in.

The other enhancements (matrix->Matrix change, SwiftOO.framework) should be in separate PRs -- they're worthy of their own discussion. This should be fairly easy to do with git's branches (from this branch, git checkout -b oo_framework) and won't have any conflicts.

stsievert commented 7 years ago

I'd still like to integrate these Swift 3 changes.

thomaskilian commented 7 years ago

I sent you a 2nd PR for that.

thomaskilian commented 7 years ago

FWIW: __conversion does no longer work in Swift see SO

stsievert commented 7 years ago

Thanks for all the work you're putting into this.