Use latest sklearn and add respective requirements#55
Conversation
heidmic
left a comment
heidmic
left a comment
There was a problem hiding this comment.
Formatting should probably have been its own PR but it's fine. Given that we have these issues with python 3.10 and cmpbayes. Can you please add a comprehensive explanation to the README and a "link" in each individual script that uses cmpbayes to make the user very aware of this? Maybe even throw an automated warning when a user tries to execute one of the scripts for stat-analysis without having cmpbayes installed that includes an explanation on what is recommended
RomanSraj
force-pushed
the
use-newest-sklearn
branch
from
b3a33ba to
c1c4eeb
Compare
| pyarrow==18.1.0 | ||
| arviz==0.21.0 | ||
| click==8.2.1 | ||
| fanova==2.0.19 # (needs: sudo apt-get install swig and sudo apt-get install python3.12-dev) |
There was a problem hiding this comment.
should we keep that extra as well as this is a rather specific use case and two extra packages system-wide might break a lot of things?
There was a problem hiding this comment.
Add some documentation what this is for. It should also require the other packages (in their original 3.10 compatible versions) so that suprb-experimentation with 3.10 is standalone
There was a problem hiding this comment.
I added a comment, but because of https://github.com/heidmic/suprb/pull/195/files#diff-fa602a8a75dc9dcc92261bac5f533c2a85e34fcceaff63b3a3a81d9acde2fc52L16 we can now not have a python3.10 environmnet where we install suprb.
In my opinion we have 2 options:
- Change https://github.com/heidmic/suprb/pull/195/files#diff-fa602a8a75dc9dcc92261bac5f533c2a85e34fcceaff63b3a3a81d9acde2fc52L16 to python_requires = >=3.10
- Have a requirements.txt only for the logging_output_scripts (and not add suprb there), that would also get rid of Use latest sklearn and add respective requirements #55 (comment), since in python3.10 the additional installs are not necessary
There was a problem hiding this comment.
I think the second route is preferable. Before settling on this, could you please check how hard upgrading cmpbayes would be?
There was a problem hiding this comment.
I forked cmpbayes to be able to change the exact pystan version, so a higher one can be used. With that I am able to install and run cmpbayes on python 3.12. For this to work, we need the following two branches:
cmpbayes: dpaetzel/cmpbayes@main...RomanSraj:cmpbayes:update-pystan
suprb: heidmic/suprb#196
If the user uses anything other than python3.10 it will generate this warning: |
RomanSraj
force-pushed
the
use-newest-sklearn
branch
from
625cca5 to
91c1629
Compare
* Add importance script * Directly use output.txt files for importance analysis * Add float mapping for f_anova * Revert "Only use essential requirements; Add 3.10 requirements (for cmpbayes)" This reverts commit 56a86d1. * Add description to importance scripts * Add same formatting as in base branch * Remove unused Ipython * Use short requirements.txt * Add file level description
| # | ||
| # Update pip with "pip install -U pip" and run it again | ||
| -e git+https://github.com/dpaetzel/cmpbayes@main#egg=cmpbayes | ||
| -e git+https://github.com/RomanSraj/cmpbayes@update-pystan#egg=cmpbayes |
There was a problem hiding this comment.
should be possible to use dpaetzel again
…e rare usage in a single one time script
heidmic
force-pushed
the
use-newest-sklearn
branch
from
94121b1 to
7ad3fab
Compare
2 participants
No description provided.