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

ScikitLearn support #4

Open
cstjean opened this issue Apr 24, 2016 · 6 comments
Open

ScikitLearn support #4

cstjean opened this issue Apr 24, 2016 · 6 comments

Comments

@cstjean
Copy link

cstjean commented Apr 24, 2016

Hi, I'm looking for algorithms to add to ScikitLearn.jl. Would you be interested in supporting the interface? I can make a PR for it. Are you going to update this package when 0.5 lands?

@trthatcher
Copy link
Owner

Hi there!

I'm definitely interested in adding support for some interfaces other than the bare-bones one I've provided. My intention with this package was to provide the model layer and wait and see how the community approaches interfaces.

However, I'm hesitant to add any dependencies at this point in time. I'm kind of waiting on JuliaLang/julia#15705. If there's a way to optionally support ScikitLearn.jl without adding any dependencies, I'm all for it.

As for updating the package for 0.5, yes definitely. I'm actually working on a rewrite on part of the back end right now that I hope to merge with master in a couple days.

@cstjean
Copy link
Author

cstjean commented Apr 27, 2016

However, I'm hesitant to add any dependencies at this point in time.

That's why I split the library between a "full-featured package", ScikitLearn.jl, and an "interface-definition package", ScikitLearnBase.jl. The latter contains one short file and no dependencies. It's the one that I REQUIRE in all the libraries I've supported so far.

There's also Requires.jl. I like the above better, but @require works too.

@cstjean
Copy link
Author

cstjean commented Apr 27, 2016

This PR is a good example. I had to add some functionality for computing conditional probabilities so it's longer than it would be for this package.

@trthatcher
Copy link
Owner

Sorry for the slow replies. I took a look through your packages - this is great work! It's nice to see progress in this area. Anyway, ScikitLearnBase is pretty lightweight, so we can just add that as a requirement.

I've made some small changes to the qda!, cda! and lda! functions (layer below exported interface). Past that it's a major rewrite of many functions. I'll be merging that in a day or two with master.... I'm just finishing up on qda! which isn't fully tested. Regroup in a couple days?

@cstjean
Copy link
Author

cstjean commented Apr 29, 2016

Sure! I wouldn't have time this week anyway.

On Fri, Apr 29, 2016 at 9:29 AM, Tim Thatcher notifications@github.com
wrote:

Sorry for the slow replies. I took a look through your packages - this is
great work! It's nice to see progress in this area. Anyway, ScikitLearnBase
is pretty lightweight, so we can just add that as a requirement.

I've made some small changes to the qda!, cda! and lda! functions (layer
below exported interface). Past that it's a major rewrite of many
functions. I'll be merging that in a day or two with master.... I'm just
finishing up on qda! which isn't fully tested. Regroup in a couple days?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@cstjean
Copy link
Author

cstjean commented Mar 8, 2017

Still interested in this? If you intend to support this package in Julia 0.6, I can make a PR for ScikitLearnBase support. I'd create new types (eg. LDAClassifier) that work on top of your current code, so there would be no modification of your code.

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

2 participants