search

rvw-org / rvw

R interface to Vowpal Wabbit
22 stars 2 forks source link

Simple checks #2

Closed eddelbuettel closed 6 years ago

eddelbuettel commented 6 years ago

For my own work, I have found that it helps me focus on bigger picture things when the "small stuff" does not distract. As R CMD check is a good habit, I think it is preferable to not fail the check.

When I run it right now, I get Status: 1 ERROR, 2 WARNINGs, 1 NOTE

The ERROR is in your unit tests, I presume you know that. That should really be fixed.

The WARNINGs are on the help pages. It is good practice to keep this moving along. Documenting an S3 method is something you need to lookup; it is not hard but it is good to go through this. Should you get stuck do not hesitate to ask us. The missing documentation should be added.

The NOTE is on use of std::cerr. Please use either REprintf or Rcpp::Rcerr as @coatless had already suggested (as I recall).

Let us know if you need help with any of this.

coatless commented 6 years ago

I think it would be really good if you also setup Travis-CI for the repository. There are two ways to go about this. You can use @eddelbuettel's travis setup or the r community setup.

eddelbuettel commented 6 years ago

Regarding std::cerr: Just commenting it out everywhere in src/ does the trick. You seem to use Rcpp::Rcout just fine, but I am not sure what you are trying to get out with the std::cerr buffer redirect. R will still see std::cerr being referenced and complain...

ivan-pavlov commented 6 years ago

Yes, thank you! I will setup Travis-CI.

Regarding that problem with std::cerr: I see now that it's not enough to just redirect output. I think I will then copy sources and comment everything there or create custom wrapper functions without any cerr/cout.

coatless commented 6 years ago

@ivan-pavlov no need to modify the vw source. This is only applicable to any defined output in a C++ file located in src/ e.g. helpers.cpp and rvw.cpp

ivan-pavlov commented 6 years ago

Yes, if I comment out everything with cout/cerr in src/ problems with R CMD check go away.

But that means functions from libvw just send output to the error stream. This doesn't look good, so I am working on a way to improve this. And maybe also avoiding modifications in libvw sources.

eddelbuettel commented 6 years ago

Yes, if I comment out everything with cout/cerr in src/ problems with R CMD check go away.

Please do, and commit that. As James and I both said before, you should leave the vw sources alone (at least for now).

eddelbuettel commented 6 years ago

This can be closed now too that we have Travis running.