immunogenomics / harmony

Fast, sensitive and accurate integration of single-cell data with Harmony
https://portals.broadinstitute.org/harmony/
Other
513 stars 98 forks source link

New version does not respect `verbose = FALSE` #211

Closed const-ae closed 9 months ago

const-ae commented 11 months ago

Hi,

the new version does not seem to respect the setting verbose = FALSE and nonetheless prints messages:

Y <- matrix(1:30, nrow = 3, ncol = 10)
group <- rep(letters[1:2], length.out = 10)
res <- harmony::RunHarmony(Y, meta_data = group, nclust = 2, verbose = FALSE)
#> Hard k-means centroids initialization

As far as I understand, it would be necessary to make the printing for example in harmony.cpp#L94 conditional on the value of verbose.


In addition, I think that the Rcout is not ideal for printing from Rcpp anyways because you cannot suppress the messages and Rcpp::message() is preferred (https://github.com/RcppCore/Rcpp/issues/1145). See the following example:

Rcpp::cppFunction(r"(
  int test(){
    Rcpp::Rcout << "hello\n";
    return 42;
  }                  

)")

suppressMessages({
  test()
})
#> hello
#> [1] 42

Rcpp::cppFunction(r"(
  int test2(){
    Rcpp::message(Rcpp::wrap("hello"));
    return 42;
  }                  
)")
suppressMessages({
  test3()
})
#> [1] 42
pati-ni commented 11 months ago

Thanks for reporting the issue. Yes, you are actually right, we will be removing this message from the c++ backend.

pati-ni commented 9 months ago

Fixed in v1.2