-
Notifications
You must be signed in to change notification settings - Fork 38
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
New primitives added -- KNN classifier & regressor #40
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a couple of things that can be removed.
However, the most important point is that there is a syntax error in the Regressor JSON.
Please correct it and later on make sure that it is valid by executing make test-all
in your virtualenv and making sure that there are no errors reported.
}, | ||
"modalities": [], | ||
"primitive": "sklearn.neighbors.KNeighborsClassifier", | ||
"validation_dataset": "wine", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line can be removed, as the validation method have changed.
}, | ||
"modalities": [], | ||
"primitive": "sklearn.neighbors.KNeighborsClassifier", | ||
"validation_dataset": "wine", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
"name": "sklearn.neighbors.KNeighborsRegressor", | ||
"author": "Sze Nga Wong (wsnalice@mit.edu)", | ||
"documentation": "http://scikit-learn.org/stable/modules/generated/sklearn.neighbors.KNeighborsRegressor.html", | ||
"description": "Regression based on k-nearest neighbors. The target is predicted by local interpolation of the targets associated of the nearest neighbors in the training set." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is missing a comma at the end, which makes the JSON syntax invalid. Can you add it?
}, | ||
"modalities": [], | ||
"primitive": "sklearn.neighbors.KNeighborsRegressor", | ||
"validation_dataset": "boston", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be removed.
Merging this in even if there are errors in it. They will be fixed afterwards in a later PR. |
This commit addresses Issue #38