Skip to content
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

Added more of the sklearn regressors to the presets #27

Merged
merged 6 commits into from
Jun 7, 2017

Conversation

enisnazif
Copy link
Contributor

Added the large majority of the more popular regressors of sklearn. I am aware that a few may be missing. Also, I tidied the code slightly and split the regressors and classifiers into two sections.

'sklearn_GP_regressor',
'sklearn_ridge_regressor',
'sklearn_lasso_regressor',
'sklearn_kernelRidge_regressor',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stylefix: probably better to use sklearn_kernel_ridge_regressor

sklearn_knn_regressor_source = \
"""from sklearn.neighbors import KNeighborsRegressor

base_learner = KNeighborsRegressor(random_state=8)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think KNN has a random_state parameter. Please also double check the other estimators to see if they have a random_state parameter.

@reiinakano
Copy link
Owner

Looks good so far! Just made a few comments that should be addressed first.

@reiinakano
Copy link
Owner

Think it looks good! Am a bit scared of pulling these without adequate unit tests, but I guess it's my fault for not writing them in the first place. I really need to get to doing those.

Thanks a lot!

@reiinakano reiinakano merged commit 14a6394 into reiinakano:master Jun 7, 2017
@enisnazif
Copy link
Contributor Author

no problem :) if you'd like, I can start having a look at writing these tests?

@reiinakano
Copy link
Owner

@enisnazif That would be very helpful! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants