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

Approximate Nearest Neighbors using Dlibml #99

Merged
merged 4 commits into from Aug 14, 2017

Conversation

Projects
None yet
3 participants
@Iron-Stark
Contributor

Iron-Stark commented Aug 2, 2017

No description provided.

@Iron-Stark

This comment has been minimized.

Show comment
Hide comment
@Iron-Stark

Iron-Stark Aug 2, 2017

Contributor

@mlpack-jenkins test this

Contributor

Iron-Stark commented Aug 2, 2017

@mlpack-jenkins test this

@rcurtin

Thanks for the hard work here. I have a handful of comments; if you can address them, I think this will be basically ready to merge.

Show outdated Hide outdated Makefile
Show outdated Hide outdated methods/dlibml/ANN.py
# Parse options into string.
optionsStr = ""
if "k" in options:
optionsStr = "-k " + str(options.pop("k"))

This comment has been minimized.

@rcurtin

rcurtin Aug 5, 2017

Member

In this case k should also be required.

@rcurtin

rcurtin Aug 5, 2017

Member

In this case k should also be required.

Show outdated Hide outdated methods/dlibml/src/ANN.cpp
Show outdated Hide outdated tests/benchmark_ann.py
@zoq

zoq approved these changes Aug 14, 2017

Looks good for me, we can ignore the build error here, it will pass with the next build.

@rcurtin rcurtin merged commit 1aa4105 into mlpack:master Aug 14, 2017

1 check failed

Benchmarks Checks Build finished.
Details
@rcurtin

This comment has been minimized.

Show comment
Hide comment
@rcurtin

rcurtin Aug 14, 2017

Member

Thanks! :)

Member

rcurtin commented Aug 14, 2017

Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment