-
Notifications
You must be signed in to change notification settings - Fork 74
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
CompatHelper: bump compat for Compat to 4, (keep existing compat) #111
CompatHelper: bump compat for Compat to 4, (keep existing compat) #111
Conversation
@cstjean @OkonSamuel Can a maintainer please take a look. This is holding back some downstream package development. |
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. 😞 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. |
Suggestions welcome. I would gladly move ScikitLearn.jl to another org if it helped. |
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. |
Yeah, that sounds reasonable. I'll see about making the discourse post. |
@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. |
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 (JuliaLang/julia#45582 (comment)), 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. |
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. |
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. |
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. |
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! |
This pull request changes the compat entry for the
Compat
package from2.2, 3
to2.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.