csinva / imodels

Interpretable ML package 🔍 for concise, transparent, and accurate predictive modeling (sklearn-compatible).
https://csinva.io/imodels
MIT License
1.36k stars 121 forks source link

Remove discretization from BRL & Python updates #90

Closed keyan3 closed 2 years ago

keyan3 commented 2 years ago

Now that we have the discretization module we no longer need to auto-discretize within BRL. Instead, we can ask users to discretize their own data using either ours or sklearn's utilities before calling BRL—this is what we do for OptimalTreeClassifier and it's probably cleaner long term (added a check for this in .fit). Fixes #81

Also, sklearns below 1.0 don't work with discretization due to changes to OneHotEncoder, which means imodels is no longer fully compatible with Python 3.6. Since 3.6 had its EOL a month ago anyway, I switched our support to 3.7 - 3.10.

csinva commented 2 years ago

Okay nice, you're right this is cleaner and all looks good to me. We might want to add a doc tutorial somewhere showing examples of discretization (+how ours differ from the standard) but we can do this later.

RIP py3.6