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
Modify NSModel to use boost variant. #693
Merged
sumedhghaisas
merged 6 commits into
mlpack:master
from
MarcosPividori:variants-for-ns-model
Jun 20, 2016
Merged
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
86a9852
Modify NSModel to use boost variant.
MarcosPividori a0bcee9
Improve name of Search visitors.
MarcosPividori ddb252c
Set SortPolicy as template parameter of classes BiSearchVisitor and T…
MarcosPividori 83f1925
Details to improve code and documentation.
MarcosPividori 6c2c3ca
Improve documentation.
MarcosPividori b34dac8
Fix NSModel serialization. Use boost serialize function for variants.
MarcosPividori File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Ah, I hate to be picky, so I hope I am not giving too much work, but do you think you could add documentation for these classes and their members and member functions? I am not sure everyone who looks through this file will be familiar with the visitor paradigm; there is no need to explain what that is in your comments, but it may be useful to have comments along the lines of "MonoSearchVisitor executes a monochromatic neighbor search on the given NSType", or something like this. Maybe a few more words are useful to explain that better. :)
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.
Actually, I should add, if you don't want to or are busy doing other things, I can do the documentation here, it will only take a few minutes.
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.
@rcurtin Sure!! I am here for this! I agree it can be confusing for someone with no knowledge of boost variants. I will add documentation.
Thanks!
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.
Done in: 6c2c3ca