Skip to content
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

Added operator<<() and print() functions to Vector #361

Merged
merged 3 commits into from
Sep 7, 2015
Merged

Added operator<<() and print() functions to Vector #361

merged 3 commits into from
Sep 7, 2015

Conversation

dcdillon
Copy link
Contributor

@dcdillon dcdillon commented Sep 1, 2015

For issue #239, added operator<<() functions for the Rcpp::Vector class. These print a space delimited list of items. In the case of a CharacterVector, each item is enclosed in ".

Additionally added a Vector::print(const std::string comment, std::ostream &stream) function at Dirk's behest. This function prints as follows:

My Comment
1 2 3 4 5 6

Unit tests are included and pass.

@eddelbuettel
Copy link
Member

(I think I said free function print() or member function print() -- idea borrowed from Armadillo.)

@dcdillon
Copy link
Contributor Author

dcdillon commented Sep 1, 2015

Whichever is fine by me. It's just moving a little code around.

@thirdwing
Copy link
Member

Just a very simple question: have you ever tried to print the Matrix class or just an element of a Matrix?

https://github.com/RcppCore/Rcpp/blob/master/inst/include/Rcpp/vector/Matrix.h#L28

@dcdillon
Copy link
Contributor Author

dcdillon commented Sep 1, 2015

Just Vectors. I can add Matrix printing, but it wouldn't use the vector pattern because with Matrix alignment would matter.

@dcdillon
Copy link
Contributor Author

dcdillon commented Sep 1, 2015

I see....because Matrix "is a" Vector. I think the proper solution here is to add a specialized Matrix printing routine then it just doesn't happen that a Matrix gets printed through the operators meant for Vectors (in the strict sense).

@kevinushey
Copy link
Contributor

Why not just implement a safe wrapper around Rf_PrintValue?

@eddelbuettel
Copy link
Member

Can we (easily) catch what Rf_PrintValue before it goes to the buffered output? Having an operator (and print() wrapper) allows for a file stream.

And yes, we surely want Matrix methods too.

@dcdillon
Copy link
Contributor Author

dcdillon commented Sep 1, 2015

Agree with @eddelbuettel that if we do it in a C++ way with streams we get the added benefits of file streams, string streams, etc.

So here's what I'm thinking for Matrix routines.

For IntegerMatrix find min/max and determine the maximum number of characters an element will take up and align based on that.

For NumericMatrix get the precision of the stream and align based on that.

For CharacterMatrix just give up on alignment because that's pretty ridiculous.

@romainfrancois
Copy link
Contributor

I agree with kevin, this is a wheel that does not need to be reinvented.

@eddelbuettel
Copy link
Member

Point taken re Rf_PrintValue being safe and all. Maybe a better idea is to wrap it?

Would still be nice to a) have operator<<() and b) a print() method with optional comment the way Armadillo does.

@dcdillon
Copy link
Contributor Author

dcdillon commented Sep 2, 2015

Then it would seem that we're out on operator<< and print should be redefined as void print(). Yes?

@eddelbuettel
Copy link
Member

Or have both after all? It is really an oversight not to offer << for a library we offer to newbs. But it is of course a good design principle to rely on R features where available.

@eddelbuettel
Copy link
Member

Ah, and @dcdillon reminds me of print() which I appear to have added six months ago...

@dcdillon
Copy link
Contributor Author

dcdillon commented Sep 2, 2015

There is one small caveat with using Rf_PrintValue. It prints, as in R, an index header at the beginning of each line:

[1] 1 2 3 4

This index is, of course, 1 based whereas usage in Rcpp of the Vector class is zero based.

So it's a little misleading (though not horrible).

@dcdillon
Copy link
Contributor Author

dcdillon commented Sep 2, 2015

Additionally, has anyone ever looked into capturing the output of Rf_PrintValue. As @eddelbuettel points out, it would be nice to have operator<<(), but this cannot happen in a sensible manner without capturing the Rf_PrintValue output and redirecting it to the proper stream.

Additionally, even if one can capture the output (temporarily) headed for Rcout, it is unlikely to be in a thread-safe manner. So technically you could be capturing some other thread's output as well.

@eddelbuettel
Copy link
Member

Fair point re thread-safeness, but there is only so and so much you can do against silly programmers. If someone invokes parallel code and printing method well then they get what they deserve...

@dcdillon
Copy link
Contributor Author

dcdillon commented Sep 5, 2015

So. There exists a free function print() that calls Rf_PrintValue. I have defined operator<< for Vector and Matrix via iterators. This is because the output of Rf_PrintValue does not seem to be able to be redirected so it makes no sense in an operator<<.

It can, of course, be changed. Just let me know what should be done.

@thirdwing
Copy link
Member

It will be very interesting if you try to print it out in gdb.

http://stackoverflow.com/questions/28073624/displaying-elements-of-rcppnumericmatrix-from-gdb

@eddelbuettel
Copy link
Member

I think for gdb use your best bet is Rf_PrintValue and if memory serves it comes up in the (old but still interesting) video tutorial by Seth (which sadly spends half its time on an issue you wouldn't have had when using Rcpp but I digress...)

@romainfrancois
Copy link
Contributor

It would be easy enough to use capture.output( print(x) ) and then print that to whatever stream afterwards

@kevinushey
Copy link
Contributor

This looks fine to me, but I still think that it would be best to implement a safe wrapper around capture.output(print(x)). I don't think Rcpp should have it's own notion how how R vectors should be printed, it should just re-use pre-existing R facilities for printing vectors.

Either way I'm comfortable accepting the PR.

@eddelbuettel
Copy link
Member

Ok, with this and the parallel email discussion this is good to go. We could of course do a capture.output(print(...)) as well. It adds consistency, but it is also lame to go back to R. Maybe I'll write a Gallery post on this. Merging now.

eddelbuettel added a commit that referenced this pull request Sep 7, 2015
Added operator<<() and print() functions to Vector
@eddelbuettel eddelbuettel merged commit f1953ac into RcppCore:master Sep 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants