Add support for callable metrics

[jgraving]

Issue

See #93

Description of changes

Added the option for callable metrics to the openTSNE.nearest_neighbors module. NNDescent checks if metric is compiled with numba, gives an informative warning, and then attempts to automatically compile it. If the function is already compiled or NNDescent is not being used then the callable is passed without any further checks. Docstrings have also been updated to match other docstrings with multiple types. If you'd like unit tests for this, then please provide guidance on how best to do this.

Includes

• Code changes
• Tests
• Documentation

[pavlin-policar]

Thanks for the PR. I am missing tests, so we verify that regular python functions properly get compiled to numba code and a test with already numba compiled code.

Also, as mentioned in #93, it would be great if we could keep things consistent, and that you would add callable support to the BallTree as well.

[jgraving]

I'll try adding tests, but I'm not very familiar and may need some guidance. BallTree doesn't overwrite the openTSNE.nearest_neighbors.KNNIndex.check_metric method, so support for callable metrics is already there.

[jgraving]

Tests are added and passing. Let me know if you want me to add something more.

[pavlin-policar]

Thanks, looks good! I've got a couple nitpicks about the empty lines, to keep the formatting and style consistent across the codebase, but otherwise looks good. Please fix these couple cosmetic changes.

[jgraving]

Should be good to go! Style errors are taken care of. I need to find a better linter. Have you seen black? https://black.readthedocs.io/en/stable/index.html I've been thinking about using it for my projects.

[pavlin-policar]

Awesome, looks great!

Have you seen black?

Yes, I'm a fan of black. Most of the code in openTSNE is compliant with its style guide, with the exception of cython files and the main tsne.py. The only reason I haven't implemented it fully is that it would introduce a whole lot more line breaks, which would IMO be less readable in this case. Otherwise, I try to use it wherever I can in my other projects.

[pavlin-policar]

I forgot to squash your commits before merging, so I had to go through some git-gymnastics to get everything down to one coherent commit. That's why there's another commit in the history here. I didn't want to bother you with this, but next time, it's better that you squash your commits into a single one with a descriptive commit message, or do an interactive rebase to organize your commits. I've taken care of it now, though. Thanks for your contribution 😄

[jgraving]

Ah, sorry about that. I'll keep that in mind for next time. No problem! Glad I could help 😄