search

config-i1 / smooth

The set of functions used for time series analysis and in forecasting.
89 stars 19 forks source link

Switch to arma_cerr_stream<T> #201

Closed eddelbuettel closed 1 year ago

eddelbuettel commented 1 year ago

Armadillo updates its coding convention over time. We have been fairly hard at work to have RcppArmadillo-using packages switch from a now-deprecated older way of instantiatig variables to a newer one (see issue #391 for details; this PR is part of the related issue #402).

When compiling packages using RcppArmadillo with the deprecation-warning-suppressor we still use, some new warnings come up. One concerns a deprecated way of setting an output or error stream as your package does in one spot in one file. Changing this is fairly straightforward, and affects only that one file.

The package compiles with the change as it did before, and test fine as well.

It would be much appreciated if you could apply this pull request and update the package at CRAN within the next few months. Let me know if you have any questions.

config-i1 commented 1 year ago

Thanks! I'll submit to CRAN ASAP.

conradsnicta commented 1 year ago

@config-i1 A better solution is to simply remove all use of arma::set_cerr_stream(). No replacement code should be necessary.

Armadillo by default prints far fewer warnings since version 10.4, so use of arma::set_cerr_stream() is not required.

More details at: https://github.com/RcppCore/RcppArmadillo/issues/402#issuecomment-1384816643

config-i1 commented 1 year ago

Okay. Does it mean that I don't need that line of code at all?

eddelbuettel commented 1 year ago

Yes, that's what it looks like. My PR replaced 'line by line' which is correct in the narrow sense, as @conradsnicta points out we can do een better with less suppression. That will likely be package dependent: you may know parameterisations that would or might trigger a warning under the old code and may no longer now.