Conversion to PythonCall

[tylerjthomas9]

Here is my initial attempt at dropping ScikitLearn.jl, and just including the base PythonCall.jl wrapper. Let me know what you think. I think that it would be great to methods for preparing the data, similar to what we did in CatBoost.jl, but I haven't implemented them yet.

[tylerjthomas9]

Fantastic work. I can't believe how quickly you pulled this together.

I note that CI is failing on 1.6 Ubuntu. My guess is that a different version of the python libraries are being installed, possible related to @OkonSamuel's libstcxx hack that has been removed??

Is there a way to restrict sk-learn version to [1.2, 1.3)?

On Julia 1.6, scikit-learn v1.1.1 is being installed due to libstdcxx restrictions. I can limit the CondaPkg.toml bounds for scikit-learn, but this would prevent v1.6 from being able to use the package. I will investigate and see if we can just patch the scikit-learn v1.1.1 issue.

[ablaom]

If you haven't done so already, perhaps you should rebase off the new dev, now that @OkonSamuel's PR is merged?

[OkonSamuel]

Fantastic work. I can't believe how quickly you pulled this together. I note that CI is failing on 1.6 Ubuntu. My guess is that a different version of the python libraries are being installed, possible related to @OkonSamuel's libstcxx hack that has been removed?? Is there a way to restrict sk-learn version to [1.2, 1.3)?

On Julia 1.6, scikit-learn v1.1.1 is being installed due to libstdcxx restrictions. I can limit the CondaPkg.toml bounds for scikit-learn, but this would prevent v1.6 from being able to use the package. I will investigate and see if we can just patch the scikit-learn v1.1.1 issue.

That's what my hack was meant to address for the CI. We could just add a readme similar to, https://github.com/cstjean/ScikitLearn.jl#known-issue

[tylerjthomas9]

If you haven't done so already, perhaps you should rebase off the new dev, now that @OkonSamuel's PR is merged?

I think that PR has been merged into my branch. I see the changes he made. I will have time after Tuesday of this week to do any remaining tasks to get this PR ready.

[codecov-commenter]

Codecov Report

Merging #56 (3840399) into dev (89227e3) will increase coverage by 7.63%. The diff coverage is 90.62%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##              dev      #56      +/-   ##
==========================================
+ Coverage   87.43%   95.07%   +7.63%     
==========================================
  Files          12       13       +1     
  Lines         207      264      +57     
==========================================
+ Hits          181      251      +70     
+ Misses         26       13      -13     

Impacted Files	Coverage Δ
src/ScikitLearnAPI.jl	80.00% <80.00%> (ø)
src/macros.jl	95.83% <90.00%> (+9.41%)	:arrow_up:
src/MLJScikitLearnInterface.jl	100.00% <100.00%> (+69.23%)	:arrow_up:
src/models/clustering.jl	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[tylerjthomas9]

I added a hack to get the CI working on Ubuntu + Julia 1.6. The hack just replaces julia's libstdcxx with the action runner's version, which is more up to date. I am not sure what is different about the github actions version of julia, but it has a older libstdcxx version than julia 1.6 when I install it on my personal computer. For me, the test suite passes with 0 issues on local julia 1.6 installs.

Let me know what you think.

[ablaom]

@tylerjthomas9 I can't see anything left to do here but I'd like @OkonSamuel to comment on your alternative hack.

Then we should coordinate this merge with the OutlierDetectionPython.jl PR. I'll open a tracking issue shortly.

[OkonSamuel]

@tylerjthomas9 I can't see anything left to do here but I'd like @OkonSamuel to comment on your alternative hack.

Then we should coordinate this merge with the OutlierDetectionPython.jl PR. I'll open a tracking issue shortly.

It's the same thing I did. i.e replacing Julia's libstdxx with the up to date system's libstdxx (In my case I replaced it with the libstdxx from the Conda package). Can we document this Hack in the README?

I added a hack to get the CI working on Ubuntu + Julia 1.6. The hack just replaces julia's libstdcxx with the action runner's version, which is more up to date. I am not sure what is different about the github actions version of julia, but it has a older libstdcxx version than julia 1.6 when I install it on my personal computer. For me, the test suite passes with 0 issues on local julia 1.6 installs.

Let me know what you think.

Are you running Ubuntu?

[tylerjthomas9]

Added a quick note in the documentation for fixing the libstdcxx issues on Linux. Let me know if I should change anything.

Are you running Ubuntu?

I am. I also tried github codespaces, and had 0 issues with Julia 1.6.7 on Linux.

[OkonSamuel]

Added a quick note in the documentation for fixing the libstdcxx issues on Linux. Let me know if I should change anything.

Are you running Ubuntu?

I am. I also tried github codespaces, and had 0 issues with Julia 1.6.7 on Linux.

That's strange. It fails when I try it on my ubuntu system. Although, I haven't ran any update on my ubuntu for a while now.