Two constructors of Vector class not meaningful

[yixuan]

Greetings Rcpp-devs,
In <Rcpp/vector/Vector.h>, I found two constructors (line 91 and 95, Rcpp version 0.11.2)

Vector( const std::string& st )
Vector( const char* st )

It seems that these are used to parse a string and convert it into a vector (of length 1, since the arguement is a single string, not a vector), possibly of numeric type, similar to as.character("123").
However, when the vector is of numeric type, these constructors will result in an error. Here is the code to reproduce the problem:

> cppFunction('
+ SEXP test()
+ {
+     return NumericVector("123");
+ }
+ ')
> test()
Error: not compatible with requested type

Tracing back the error, I found that the constructors here called
internal::vector_from_string<REALSXP>(st) (<Rcpp/vector/vector_from_string.h>, line 28), which called
r_cast<REALSXP>(str) (<Rcpp/r_cast.h>, line 138), which called
r_true_cast<REALSXP>(x) (<Rcpp/r_cast.h>, line 69), which called
basic_cast<REALSXP>(x) (<Rcpp/r_cast.h>, line 49).
In the last function, x is of type STRSXP, so it will reach the default section of the switch statement, which just throws an error of the message shown in the example.
In fact, the R API function Rf_coerceVector(X, RTYPE) does support the conversion from string to numeric, so it seems reasonable to add STRSXP support.

[eddelbuettel]

Nice work, thanks. So where do we go from here? Do you want to prepare a PR adding the additional case for string conversion?

[yixuan]

It should be trivial to just add a case in the switch statement, but I'm not sure if it will break the structure of Vector, since if string is not convertible to numbers, it will result in NA, and give a warning.
Are these constructors used somewhere else in Rcpp?

[eddelbuettel]

It's hard to tell. If we don;t add the switch, we could deprecate the constructor by putting a big fat warning in now, and removing them in due course, say a year.

[kevinushey]

I would prefer that the code just didn't compile (ie, I think we should remove those ctors) -- I think it's better to be explicit when asking for conversion from string to numeric (and the reverse), and IMO the Vector constructor isn't the best place to do it.

So, for example, someone could write the (verbose):

NumericVector v = NumericVector::create(std::stod(s)); // assuming C++11 support
NumericVector v = NumericVector::create(atof(s.c_str())); // older support 

Or we could add some utility function like:

NumericVector v = from_string(s); // a reversal of the std::to_string interface from C++11

which converts its string argument to the appropriate type.

Thoughts?

On Sat, Aug 30, 2014 at 11:46 AM, Dirk Eddelbuettel
notifications@github.com wrote:

It's hard to tell. If we don;t add the switch, we could deprecate the
constructor by putting a big fat warning in now, and removing them in due
course, say a year.

—
Reply to this email directly or view it on GitHub.

[eddelbuettel]

Immediately, or after a deprecation period?

[kevinushey]

After a deprecation period, just to be safe.

[eddelbuettel]

Agreed, putting that in right now in a branch.

[kevinushey]

Oops. A reason why we might not want to deprecate these constructors -- they are sensible for CharacterVectors, e.g. it allows:

CharacterVector x = std::string("foo"); // creates a length one character vector

although we would likely prefer using the Rcpp::String class in these instances anyhow; regardless, surely there are packages depending on that behaviour.

[eddelbuettel]

Ooops. I revert the deprecation notice then.

@yixuan: With that we are back to the switch idea to actually making it work, correct?

[yixuan]

Possibly, or just leave it as it is, since the problem is an R runtime error rather than compilation error. Just let users know that these constructors are intended to work with CharacterVector.
Any other idea?

[yixuan]

Actually I'm working on a function-by-function reference of Rcpp API (unofficially of course :D),
and here is how I document these two functions at the moment. It would be helpful if Rcpp has some official reference guide in the future to let users know how they are supposed to use the functions. What I'm doing now may serve as a preliminary step.

[kevinushey]

This is wonderful! Thanks for putting this together and please ask on the mailing list if you discover anything else ambiguous / difficult to understand; I think this could be a very useful resource.