scholar
scholar copied to clipboard
Refactor Scholar.Neighbors
As argued in #254, we need to update Scholar.Neighbors
in the following way:
- [x] Add
BruteKNN
module that implements brute-force k-NN search. Itspredict
function has to return{neighbor_indices, neighbor_distances}
. Makebatch_size
an option (as in #253). - [ ] Implement
KNNClassifier
. - [ ] Implement
KNNRegressor
. - [ ] Deprecate
Scholar.Neighbors.KNearestNeighbors
.
KNNClassifier
and KNNRegressor
have to take k-NN algorithm and metric options as atoms, but also allow for custom k-NN algorithms and metrics to be passed as modules and functions, respectively.
We don't need to deprecate, we can just rename the module. It should be easy for anyone to just rename it accordingly. :)
Hi @krstopro, I want to release a new version soon. If we plant to remove Scholar.Neighbors.KNearestNeighbors
, it would be better to do it before, so we update the docs accordingly?
We would also need to mirror those decisions on top of RadiusNearestNeighbors
.
Hi @krstopro, I want to release a new version soon. If we plant to remove
Scholar.Neighbors.KNearestNeighbors
, it would be better to do it before, so we update the docs accordingly?
@josevalim Agreed. I will try to implement the stuff from this issue by the end of the week if that's fine.
We would also need to mirror those decisions on top of
RadiusNearestNeighbors
.
Correct, shouldn't be hard.
Perfect. KNN is the most important, Radius can be done later. :)
Thank you @krstopro! I will work on #265 and @msluszniak will update the notebooks. :) Once KNNRegression is ready, we can release a new version!