This looks good. I had in the back of my mind a though that R'smedian() had a bazillion further options, but maybe I was thinking aboutquantile() and its (crazy but impressive)type= argument.

So this is probably good too but one or two seconds would be nice.

Okay, that sounds good. In the meantime I will make the appropriate changes regarding your comments above. Is this the newer version of the formatting line you referred to:

-*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*-

Yep. Hard tabs are evil, which we learned eventually. But I leave commits touching every single file to@kevinushey :) -- in short, bulk changes aren't worth it either.

There isn't too much about code style you don't already know or do. Four spaces is better than too, indentation pretty much like R Core and other sensible people do etc pp. No biggies.

Agreed on the spaces vs. tabs - it was just sloppy copy & paste on my part from another sugar file, and being avim guy (sorry!) the outdated formatting settings didn't catch my eye.

FWIW we now use.dir-locals.el to enforce indentation rules when editing with Emacs; does Vim have an analogue for directory-local variables / indentation rules?

Maybe we can also have a.vim.custom to set indent.

But if we really want to have a consistent coding style, we might think about cpplint from Google.

Re.dir-locals.el, the fanciest / most general scheme I know is viaeditorconfig where, as I understand it, you define a 'policy' or outcome and the editor-specific plugins implement it in the editor of choice.

Alternatively, and there clang-format but all of that is a bit of the heavy-handed side.

My bad for the close and re-open. Hit the wrong button.

Other than some minor comments this looks good to me.

@kevinushey Thanks for the feedback - I do believe it is necessary to callRcpp::clone in the constructor as above. I just did a rebuild without the use ofclone and it appears to modify the original vector:

#include<Rcpp.h>// [[Rcpp::export]]doublemedian_dbl(Rcpp::NumericVector x,bool na_rm =false) {returnRcpp::median(x, na_rm);}// [[Rcpp::export]]doublemedian_int(Rcpp::IntegerVector x,bool na_rm =false) {returnRcpp::median(x, na_rm);}// [[Rcpp::export]]Rcomplexmedian_cx(Rcpp::ComplexVector x,bool na_rm =false) {returnRcpp::median(x, na_rm);}// [[Rcpp::export]]Rcpp::Stringmedian_ch(Rcpp::CharacterVector x,bool na_rm =false) {returnRcpp::median(x, na_rm);}/*** Rset.seed(123)(xx <- rnorm(5))#[1] -0.56047565 -0.23017749  1.55870831  0.07050839  0.12928774median_dbl(xx)#[1] 0.07050839xx#[1] -0.56047565 -0.23017749  0.07050839  0.12928774  1.55870831set.seed(123)(xx <- as.integer(rpois(5, 20)))#[1] 17 25 12 20 27median_int(xx)#[1] 20xx#[1] 17 12 20 25 27set.seed(123)(xx <- rnorm(5) + 2i)#[1] -0.560476+2i -0.230177+2i  1.558708+2i  0.070508+2i  0.129288+2imedian_cx(xx)#[1] 0.070508+2ixx#[1] -0.560476+2i -0.230177+2i  0.070508+2i  0.129288+2i  1.558708+2iset.seed(123)(xx <- sample(letters, 5))#[1] "h" "t" "j" "u" "w"median_ch(xx)#[1] "t"xx##[1] "h" "j" "t" "u" "w"*/

As for the other points, I'll setRTYPE2 toRESULT_RTYPE and swap out theswitch statement with anif statement and commit the changes.

I have not used / am not aware of anything analogous to.dir-locals.el or the Emacs configuration lines embedded in the Rcpp headers; I just have the standard~/.vimrc and~/.vim/ftplugin/*.vim files on my machines.

Time to pull this one in!

👍

No, thumbs up toyou -- that was really well done (but I was a little tied up yesterday and the day before).