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

@describeIn fails when used on S3 method and other funs together #1181

Closed
maxheld83 opened this issue Jan 11, 2021 · 1 comment · Fixed by #1182
Closed

@describeIn fails when used on S3 method and other funs together #1181

maxheld83 opened this issue Jan 11, 2021 · 1 comment · Fixed by #1182
Labels
feature a feature request or enhancement rd ✍️

Comments

@maxheld83
Copy link
Member

This was already partially addressed #666. (It was probably only partially addressed because I had failed to provide a reprex).

As per #666 and the below test, you can now document other functions via @describeIn for generics:

test_that("all other combinations fallback to function list", {
out <- roc_proc_text(rd_roclet(), "
#' Generic
foo <- function(x) UseMethod('foo')
#' @describeIn foo related function
bar <- function(y) {}
")[[1]]
expect_equal(out$get_value("minidesc")$type, "function")
})

However, this fallback isn't triggered when you @describeIn methods and other functions together:

library(roxygen2)

  out <- roc_proc_text(rd_roclet(), "
  #' Generic
  # foo <- function(x) {}
  foo <- function(x) UseMethod('foo')

  #' @describeIn foo some method
  as.character.zap <- function(x) c('zap')
  
  #' @describeIn foo related function
  bar <- function(y) {}
")[[1]]

yields:

Error in merge.rd_section_minidesc(self$get_section(type), section) : 
  identical(x$value$type, y$value$type) is not TRUE

Whether the "parent" function (the function you're @describeIn in) is a normal function or a generic does not seem to matter.
You can trigger the same error with:

library(roxygen2)

  out <- roc_proc_text(rd_roclet(), "
  #' Normal
  foo <- function(x) {}

  #' @describeIn foo some method
  as.character.zap <- function(x) c('zap')
  
  #' @describeIn foo related function
  bar <- function(y) {}
")[[1]]

Motivation

I think this scenario might sometimes happen when documenting new (S3) OO schemes.
For example, you might have a class, and wish to document some (say, print) methods with it, as well as some "normal" functions, such as a validator.

More Background

(Not necessary to debug this, but might be helpful to other people who run into this and are confused about the relationship to vctrs::s3_register()

You can also run into this problem if you wish to expand other generics (say, knitr::knit_print(), which you don't want to @ImportFrom (and therefore Imports: in your DESCRIPTION).
If you then place knitr in your Suggests: and dynamically vctrs::s3_register() knit_print in your onLoad, your newly minted method knit_print.foo() will then look (to roxygen anyway) as a normal function, and will therefore trigger the above error.

@maxheld83 maxheld83 changed the title cannot use @describeIn for S3 method and other funs @describeIn fails when used on S3 method and other funs together Jan 11, 2021
maxheld83 added a commit to maxheld83/roxygen that referenced this issue Jan 11, 2021
@hadley hadley added feature a feature request or enhancement rd ✍️ labels Apr 16, 2021
@hadley
Copy link
Member

hadley commented Jul 10, 2022

This now gives

library(roxygen2)

  out <- roc_proc_text(rd_roclet(), "
  #' Normal
  foo <- function(x) {}

  #' @describeIn foo some method
  as.character.zap <- function(x) c('zap')
  
  #' @describeIn foo related function
  bar <- function(y) {}
")[[1]]
#> Warning: [<text>:9] Don't know how to combine @describeIn types "class" and
#> "function"

Created on 2022-07-10 by the reprex package (v2.0.1)

And is a duplicate of #1370

@hadley hadley closed this as completed Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement rd ✍️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants