cstjean / ScikitLearn.jl

Julia implementation of the scikit-learn API https://cstjean.github.io/ScikitLearn.jl/dev/
Other
547 stars 75 forks source link

CompatHelper: bump compat for Compat to 4, (keep existing compat) #111

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 2 years ago

This pull request changes the compat entry for the Compat package from 2.2, 3 to 2.2, 3, 4. This keeps the compat entries for earlier versions.

Note: I have not tested your package with this new compat entry. It is your responsibility to make sure that your package tests pass before you merge this pull request.

ablaom commented 1 year ago

@cstjean @OkonSamuel Can a maintainer please take a look. This is holding back some downstream package development.

cstjean commented 1 year ago

ScikitLearn.jl isn't in a great state ATM, but I don't have time for it. I merged, but I'm guessing that we can't tag a release without getting the tests to pass, which is probably a lot of work. :disappointed: Not sure what to do.

I really wish we could freeze the supported scikit-learn version. This would be the easiest way forward, to alleviate the support burden, but I'm not sure if Conda.jl allows it. IIRC the latest replacement for PyCall does allow this.

cstjean commented 1 year ago

Suggestions welcome. I would gladly move ScikitLearn.jl to another org if it helped.

cstjean commented 1 year ago

The good news is that a lot of the tests pass, so presumably this boils down to removing support for Julia 1.0, and seeing what's up with ubuntu-latest.

image

ablaom commented 1 year ago

😞 Not sure what to do.

Sadly, we're spread pretty thin at MLJ and don't really have experience wrapping python and all the pkg mgmt hassles around that.

Perhaps you can post a plea for help on Julia discourse? If you ping me, I can say something about MLJ depending on this package to expose sk-learn models. I admit I have been kicking the can on this one for a while. The reality is that MLJ will ultimately have to drop sk-learn support if this nice packages is not maintained.

cstjean commented 1 year ago

The reality is that MLJ will ultimately have to drop sk-learn support if this nice packages is not maintained.

Yeah, that sounds reasonable. I'll see about making the discourse post.

ablaom commented 1 year ago

@tylerjthomas9 You have experience wrapping python code. Any idea why ubuntu latest might be failing here? If you have a chance to have a wee look.

tylerjthomas9 commented 1 year ago
  ImportError("/opt/hostedtoolcache/julia/nightly/x64/bin/../lib/julia/libstdc++.so.6: version `GLIBCXX_3.4.30' not found (required by /home/runner/.julia/conda/3/lib/python3.10/site-packages/scipy/optimize/_highs/_highs_wrapper.cpython-310-x86_64-linux-gnu.so)")

This looks like an issue with Julia's GCC version. GCC 12 has just been merged into the master branch for julia (https://github.com/JuliaLang/julia/pull/45582#event-7730373179), and it is labeled for backporting to 1.6. However, I am not sure how long it will take. There is also a PR that could potential fix this from happening in the future.

I will look into it more tomorrow. Ideally, we could just limit the scikit-learn version (I am not sure how to limit package versions with PyCall.jl) to the latest version that doesn't break the build until a longer term solution is in place.

cstjean commented 1 year ago

Ideally, we could just limit the scikit-learn version (I am not sure how to limit package versions with PyCall.jl) to the latest version that doesn't break the build until a longer term solution is in place.

Maybe https://github.com/JuliaPy/Conda.jl could help with that. It's already used in ScikitLearn, but it looks like it supports environments now.

https://github.com/cjdoris/PythonCall.jl also supports envs, but that would be a very disruptive change.

tylerjthomas9 commented 1 year ago

I think that switching to PythonCall.jl would be extremely beneficial. This package seems quite popular according to the JuliaHub statistics, so it is probably worth the investment. If you are open to a large change such as PyCall -> PythonCall, I will look into it.

I submitted a PR with the same temporary workaround that PythonCall.jl uses for this issue.

cstjean commented 1 year ago

https://github.com/cjdoris/PythonCall.jl/graphs/contributors doesn't look super active, but probably good enough. I think I'd like to get more opinions on discourse before switching. Will try to post something.

cstjean commented 1 year ago

I think that switching to PythonCall.jl would be extremely beneficial. This package seems quite popular according to the JuliaHub statistics, so it is probably worth the investment. If you are open to a large change such as PyCall -> PythonCall, I will look into it.

After pondering this for a bit, I'd say that the most important question is about continuity and maintenance. If you are interested in doing this work, and maintaining it in the future, then I would gladly merge the PR and make you co-maintainer.

Beware: ScikitLearn.jl is a literal translation of the Python code from 6 years ago or so. Not the prettiest thing!