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

inheritDotParams includes duplicate param definitions if param already documented #885

Closed
mjskay opened this issue Jul 24, 2019 · 3 comments
Closed

Comments

@mjskay
Copy link
Contributor

@mjskay mjskay commented Jul 24, 2019

In the (I think) common case where one wants to make a wrapper function (say wrapper) around an existing function (original) where some (but not all) parameters from original are set to particular values in wrapper, I often see (and am currently using) a pattern of overriding just those parameters and forwarding everything else with .... Like this:

#' Wrapper around original
#'
#' @inherit original
#' @inheritDotParams original
#' @param y some more specific description
#' @export
wrapper <- function(x = "some_value", y = "some other value", ...) {
  original(x = x, y = y, ...)
}

#' Original function
#'
#' @param x x description
#' @param y y description
#' @param z z description
#' @export
original <- function(x, y, z, ...) {}

I think this is a good case for @inherit + @inheritParams simultaneously, and is related to but separate from #857 --- I'm currently encountering both problems in writing function wrappers. The issue is that @inheritDotParams includes parameters in its listing under ... even if they are already defined in the function (either because of a @param usage or an @inherit usage). Thus I get this as output:

\arguments{
\item{x}{x description}

\item{y}{some more specific description}

\item{...}{Arguments passed on to \code{original}
\describe{
  \item{x}{x description}
  \item{y}{y description}
  \item{z}{z description}
}}
}

But would expect the output below instead, since x nor y are already defined by the function and so cannot be passed through ...:

\arguments{
\item{x}{x description}

\item{y}{some more specific description}

\item{...}{Arguments passed on to \code{original}
\describe{
  \item{z}{z description}
}}
}

I know that I can get this output by adding -x -y to the @inheritDotParams spec, but it seems like this could be detected automatically without any false positives, and it would simplify documentation maintenance quite a bit in cases where there is more than just a parameter or two (I am currently working on some geoms that have an exclusion list of ~8 params, all of which I think it could exclude automatically).

@x1o
Copy link

@x1o x1o commented Jun 2, 2021

In this example, if you remove @inherit original from wrapper(), then the following arguments description is generated:

y       some more specific description
...     Arguments passed on to original
        x  
            x description
        z
            z description

whereas only z is effectively passed to original in ....

Furthermore, if wrapper() has parameters that original() does not, and we explicitly provide arguments for those parameters while calling original(), then they show up in the dots section anyway, e.g.:

#' Wrapper around original
#'
#' @inherit original
#' @inheritDotParams original
#' @param y some more specific description
#' @export
wrapper <- function(x = 'some_value', y = 'some other value', ...) {
    original(w = 'w', x = x, y = y, ...)
}

#' Original function
#'
#' @param x x description
#' @param y y description
#' @param z z description
#' @param w w description
#' @export
original <- function(w, x, y, z, ...) {}

yields

x       x description
y       some more specific description
...     Arguments passed on to original
        z
            z description
        w  
            w description

@mjskay
Copy link
Contributor Author

@mjskay mjskay commented Jun 2, 2021

In this example, if you remove @inherit original from wrapper(), then the following arguments description is generated:

y       some more specific description
...     Arguments passed on to original
        x  
            x description
        z
            z description

whereas only z is effectively passed to original in ....

I believe this would fail R CMD CHECK because x is provided as a parameter but not documented, which would be corrected by either documenting x manually or through @inherit (or I suppose removing x as an explicit parameter to wrapper()).

Furthermore, if wrapper() has parameters that original() does not, and we explicitly provide arguments for those parameters while calling original(), then they show up in the dots section anyway, e.g.:

I believe the solution for this example is to add -w to the @inheritDotParams call. Since ... can be used any number of ways (and in multiple places) inside the function I doubt there's an automatic way to detect that case.

@x1o
Copy link

@x1o x1o commented Jun 3, 2021

Eh, okay :) Still feels like it could be automated, but yeah, myself I end up writing -w too.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants