cesium-ml / cesium

Machine Learning Time-Series Platform
Other
670 stars 101 forks source link

Model selection #212

Closed arkwave closed 7 years ago

arkwave commented 7 years ago

Updated sklearn.grid_search to sklearn.model_selection as per #138. Underlying code does not appear to need much changing, but please let me and @alejandrogarciasalas know if this breaks anything.

bnaul commented 7 years ago

This won't work with older versions of scikit-learn: an easy way to stay backwards-compatible would be

try:
    from sklearn.model_selection import GridSearchCV
except:
    from sklearn.grid_search import GridSearchCV
arkwave commented 7 years ago

@bnaul sounds good, I'll make that change right away. Do you see anything else that might be problematic?

stefanv commented 7 years ago

@bnaul Are you happy with this approach, or would you like to see it refactored into something like from sklearn_compat import ...?

bnaul commented 7 years ago

I think this is the right way to do it; adding a new layer just to handle this 1 piece of logic feels too heavy to me. If we run into any other similar issues then it might make sense to combine but for now 👍

stefanv commented 7 years ago

Good job, @arkwave & @alejandrogarciasalas