<div dir="ltr">Understood!<div><br></div><div>I will contact Ryan and modify the package in the weekend.</div><div><br></div><div>Best,</div><div><br></div><div>KK</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Wed, Jun 11, 2014 at 9:28 AM, Dirk Eddelbuettel <span dir="ltr"><<a href="mailto:edd@debian.org" target="_blank">edd@debian.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
On 11 June 2014 at 08:50, Qiang Kou wrote:<br>
| Thanks for the opinions!<br>
<br>
As they say, "talk is cheap" so I dispense it freely :)<br>
<div class=""><br>
| On Wed, Jun 11, 2014 at 7:38 AM, Dirk Eddelbuettel <<a href="mailto:edd@debian.org">edd@debian.org</a>> wrote:<br>
|     And smart how you just added the little bit from Boost<br>
|     we don't have in BH (program_options).<br>
|<br>
| As far as I know all parts in BH package are headers-only, and<br>
| "program_options" is required by MLPACK to handle command line options. As I<br>
| see, it will not be used, since we don't need those options when calling it in<br>
| R.<br>
<br>
</div>Correct.<br>
<br>
So the best way forward would be to talk to Ryan to ifdef these parts, or<br>
maybe deal with it at your end so that file containing options parsing is not used.<br>
<br>
Not including it would be even better as you'd avoid all possible version<br>
skew between BH and your included file. <br>
<div class=""><br>
|     Should the cpp files in inst/include be in src/, or maybe src/mlpack<br>
|     instead?<br>
|<br>
| To be frank, I don't know which place is better. I just follow the structures<br>
| of RcppArmadillo.<br>
<br>
</div>Not really. Look for carefully: directory inst/include/ for Rcpp,<br>
RcppArmadillo, RcppEigen, BH, ... only include __header__ files whereas you<br>
included source files (ie .cpp).<br>
<div class=""><br>
|     It may be less than ideal that the kmeans example hides the base function.<br>
|     Maybe make it mlKmeans, or keep it unexported, or ... ?<br>
|<br>
| Of course, I will change names of functions, at least not the same with R<br>
| built-in functions.<br>
<br>
</div>Yes. And one fun thing you could play with is to see if you either re-use or<br>
mimic the print, summary, ... methods for kmeans.<br>
<br>
A very good start, and good to gave on GitHub!<br>
<span class="HOEnZb"><font color="#888888"><br>
Dirk<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
--<br>
<a href="http://dirk.eddelbuettel.com" target="_blank">http://dirk.eddelbuettel.com</a> | @eddelbuettel | <a href="mailto:edd@debian.org">edd@debian.org</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Qiang Kou<div><a href="mailto:qkou@umail.iu.edu" target="_blank">qkou@umail.iu.edu</a><br><div>School of Informatics and Computing, Indiana University</div>
<div><br></div></div></div>
</div>