poissonconsulting / nlist

An R package to create and check numeric list objects
https://poissonconsulting.github.io/nlist/
Other
6 stars 1 forks source link

Why can't export `coda::as.mcmc.list`? #35

Open joethorley opened 3 years ago

joethorley commented 3 years ago

This

#' @export
coda::as.mcmc.list

gives

N  checking S3 generic/method consistency (783ms)
   Found the following apparent S3 methods exported but not registered:
     as.mcmc.list
   See section ‘Registering S3 methods’ in the ‘Writing R Extensions’
   manual.
joethorley commented 1 year ago

Apparently just have to live with the NOTE but this is unsatisfying

https://stat.ethz.ch/pipermail/r-package-devel/2018q1/002476.html

krlmlr commented 1 year ago

A similar problem might have been solved already in vctrs:

https://github.com/r-lib/vctrs/blob/fb238a14a7c7e602fc81365ea29b459f128b17c1/R/type-bare.R#L6-L9

joethorley commented 1 year ago

It's the generic I'm struggling to export without a note.

I can export the methods I've implemented no problem.

#' @method as.mcmc.list nlists
#' @export
as.mcmc.list.nlists <- function(x, ...) {
  deprecate_warn("0.2.1", "as.mcmc.list()", "as_mcmc_list()", id = "as_mcmc_list")
  as_mcmc_list(x, ...)
}

As I'm deprecating its not a big deal but it would be great to find a work around for future.

krlmlr commented 1 year ago

The NOTE will cause headaches when submitting to CRAN.

This is the reason why it works with coda:

tools::nonS3methods("coda")
#> [1] "as.mcmc.list"

Created on 2023-05-19 with reprex v2.0.2

R has this info hard-coded, but it doesn't apply when we reexport. We could submit a patch to R-devel to make it work.

On a side note, I wonder if it's necessary to import coda in nlist.

joethorley commented 1 year ago

I would definitely be interested in working with you to submit a patch to R-devel.

joethorley commented 1 year ago

I think its worth importing coda as it a lightweight packages (1 dependency lattice) that provides the thin generic and defines the mcmc and mcmc.list S3 classes which are by far the most common ways to store MCMC values (its really useful to be able to convert back and forwards from these to nlist objects).

krlmlr commented 1 year ago

Patch prepared at https://github.com/r-devel/r-svn/pull/127. The check results there should inform us if it introduces any unwanted side effects.

krlmlr commented 1 year ago

Checks are green, we can use https://patch-diff.githubusercontent.com/raw/r-devel/r-svn/pull/127.patch to submit the patch via Bugzilla.

joethorley commented 1 year ago

Noice!