scholar

scholar copied to clipboard

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. Its predict function has to return {neighbor_indices, neighbor_distances}. Make batch_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!