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

Deprecating old stderr stream setting code in packages using RcppArmadillo #402

Closed
7 tasks done
eddelbuettel opened this issue Jan 15, 2023 · 9 comments
Closed
7 tasks done

Comments

@eddelbuettel
Copy link
Member

eddelbuettel commented Jan 15, 2023

Issue #391 works towards no longer suppressing a deprecation warning for use of a very old scheme to instantiate variables. When we no longer suppress these warnings we also see a number of packages exhibting a deprecation warning such as

warning: ‘void arma::set_cerr_stream(std::ostream&)’ is deprecated

which is easy to address via an updated, not deprecated call. This issue regroups the stautus of pull requests (or emailed patches) for the packages involved:

@eddelbuettel
Copy link
Member Author

Ah well it would have helped if you had documented that earlier / clearer. Feel free to alter the six PRs I wrote yesterdat to avoid the deprecation noise coming from what you once posted as documented (as users apparently found that in your logging section).

I am not planning to unmask the deprecation warning for maybe two or three months so there is time to clean this up. Contributions welcome.

@eddelbuettel
Copy link
Member Author

eddelbuettel commented Jan 17, 2023

It's not part of the public API, so it should never be placed in user code.

Conrad, please. It is in the same header file as the functions barking. If you do not highlight what function to use instead of a deprecated one, you cannot be upset that us peons take one you do not like. Document it better to help us!

Please look at the six (out of over 1000 !!) packages using RcppArmadillo having this deprecation issue, study how they use the deprecated cerr stream setter and maybe we can come up with a better way between now and April (my target date for removing the deprecation muffler). At the end of the day all six were already using a kludge to suppress a likely meaningful warning so maybe we should take this time and dig deeper.

Every CRAN package foo can be inspected online via the GitHub mirror at github.com/cran/foo i.e. each package is a mirrored repo in the cran organisation.

(Doing deprecation well is hard !! We have the same issue at work in the (core C++) library tiledb I wrap in the tiledb-r package. Pointing to promoted alternative is as best as I can tell the only viable alternative.)

@bstewart
Copy link

@conradsnicta Thanks for weighing in and for Armadillo!

I am happy to take out the internal code and replace with the macro as you recommend.

For @eddelbuettel with respect to your comment:

At the end of the day all six were already using a kludge to suppress a likely meaningful warning so maybe we should take this time and dig deeper.

I just wanted to document here that the warning definitely is meaningful, we knew the situation could arise and test for it and address ourselves—the issue was that the fastest way to test was to attempt the decomposition and have it fail which produced a warning that (10 years ago) we couldn't shut off (but now we can!). Part of the challenge is that I'm more of a statistician than a programmer (and definitely more of an R programmer than a C++ programmer). It is a testament to both your efforts to make RcppArmadillo easy to use that I was able to integrate it into stm at all.

Anyways, I wanted to thank you both so much for fundamental contributions that really made stm possible. I'll get this updated as soon as I can and up on CRAN.

@eddelbuettel
Copy link
Member Author

Oh yes of course -- we also need to take care of error conditions. But sometimes it is nice to do it locally:

> message("Some loud stuff")      # messaging users is good and needed at times
Some loud stuff
> suppressMessages(message("Some loud stuff"))     # peace and quiet as desired
> 

@eddelbuettel
Copy link
Member Author

As of 2023-02-04:

> suppressMessages(library(data.table))
> pkgs <- c("Rphylopars", "rxode2", "stm")
> db[Package %in% pkgs,.(Package,Version,Date,Published)][order(-Published)]
      Package Version       Date  Published
       <char>  <char>     <char>     <char>
1:     rxode2  2.0.11       <NA> 2022-11-01
2: Rphylopars   0.3.9 2022-05-08 2022-05-09
3:        stm   1.3.6       <NA> 2020-09-18
> 

@eddelbuettel
Copy link
Member Author

eddelbuettel commented Mar 12, 2023

Unchanged 2023-03-12:

> suppressMessages(library(data.table))       
> db <- data.table(tools::CRAN_package_db())
> pkgs <- c("Rphylopars", "rxode2", "stm")      
> db[Package %in% pkgs,.(Package,Version,Date,Published)][order(-Published)]         
      Package Version       Date  Published              
1:     rxode2  2.0.11       <NA> 2022-11-01             
2: Rphylopars   0.3.9 2022-05-08 2022-05-09      
3:        stm   1.3.6       <NA> 2020-09-18                 
>  

@eddelbuettel
Copy link
Member Author

eddelbuettel commented Sep 17, 2023

Still present in Rphylopars and stm as of 2023-09-17.

eddelbuettel added a commit to eddelbuettel/r2sundials that referenced this issue Nov 1, 2023
For reasons related to issues discussed in RcppCore/RcppArmadillo#391 and RcppCore/RcppArmadillo#402, RcppArmadillo is building with deprecation warnings suppressed.  I plan to change that ... and when I do r2sundials with create three warnings from your use of a three-argument form of `insert_cols` that is now deprecated.  This is simple: use the two-argument form.  I tested the change and R CMD check still passes.

It would be great if you could update r2sundials in the next few weeks. I plan to update RcppArmadillo in about a month and then stop suppressing warnings.
@eddelbuettel
Copy link
Member Author

RcppArmadillo release 0.12.6.6.1 is now on CRAN, and no longer defines ARMA_IGNORE_DEPRECATED_MARKER.

@eddelbuettel
Copy link
Member Author

The remaining package is now off CRAN meaning the transition is now complete as is #391. Many thanks to all maintainer who updated their packages.

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

No branches or pull requests

2 participants