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

R6 class inheriting from unexported parent causes check() error #1236

Closed
hongooi73 opened this issue Jun 22, 2021 · 6 comments · Fixed by #1393
Closed

R6 class inheriting from unexported parent causes check() error #1236

hongooi73 opened this issue Jun 22, 2021 · 6 comments · Fixed by #1393
Labels
bug an unexpected problem or unintended behavior R6 6️⃣

Comments

@hongooi73
Copy link

Running devtools::check() on an internal package gives the following result:

> checking Rd cross-references ... WARNING
  Missing link or links in documentation object 'LiquiditySeekerStrategy.Rd': 
    '[algotrade:Strategy]{algotrade::Strategy}'

  Missing link or links in documentation object 'TWAPStrategy.Rd':
    '[algotrade:Strategy]{algotrade::Strategy}'

  See section 'Cross-references' in the 'Writing R Extensions' manual.        

The reason, as far as I can tell, is because my exported TWAPStrategy and LiquiditySeekerStrategy classes inherit from a parent Strategy class, which is private. My documentation markup for the exported classes doesn't include any explicit links to Strategy, but it appears that roxygen2 inserts them anyway.

Some sample markup:

#' TWAP strategy class
#'
#' @description
#' This class emulates the TWAP trading stategy.
#' @details
#' The TWAP strategy is the simplest strategy in use. (...)
#' @export
TWAPStrategy <- R6::R6Class("TWAPStrategy", inherit=Strategy, ...)

And the start of the corresponding .Rd file:

% Generated by roxygen2: do not edit by hand
% Please edit documentation in R/TWAPStrategy.R
\name{TWAPStrategy}
\alias{TWAPStrategy}
\title{TWAP strategy class}
\description{
This class emulates the TWAP trading stategy.
}
\details{
The TWAP strategy is the simplest strategy in use. (...)
}
\section{Super class}{
\code{\link[algotrade:Strategy]{algotrade::Strategy}} -> \code{TWAPStrategy}
}

Notice the link to the Strategy class, which check() complains about. Should this link be there for an unexported class?

@gaborcsardi
Copy link
Member

If you add @aliases Strategy to the TWAPStrategy class, the warning probably goes away.

@bmoretz
Copy link

bmoretz commented Nov 23, 2021

@gaborcsardi is there any other possible workaround for this? I've spent the better part of the day trying to make this warning go away in the exact same scenario.

I've tried: @aliases [Base] (from Derived) and @aliases [Derived] (from base), both result in another warning:

Rd files with duplicated alias 'Base'

Tried using @seealso \code{"\link[=Base]{Base}"}, @include base.R, @family [Base], etc. etc.

I've even tried overriding @sections Super class:, but I just end up with 2 copies of that section in the .Rd file and more warnings.

How can you explicitly set what gets set in in the \section{Super class} section? no matter what I try I always end up with this getting generated:

\section{Super class}{
\code{\link[pkgname::NA]{pkgname::NA}} -> \code{Dervied}
}

It doesn't appear that this section is optional for roxygen with types that inherit from another R6, as ?Dervied always shows a section:

Super class:
pkgname::NA -> Derived

which results in:

Missing link or links in documentation object 'File.Rd':
    ‘[pkgname:NA]{pkgname::NA}’

1 warning for every occurrence of a derived type in the package.

I would greatly appreciate any possible clues or hints,

@gaborcsardi
Copy link
Member

@bmoretz Can you show your package so we can try this?

@bmoretz
Copy link

bmoretz commented Nov 23, 2021

@gaborcsardi absolutely, thank you. shiny.gentelella

Just remove the tools/check.env, as I temporarily suppressed it so I could move on.

Element is the ABC, Control/Page/Dashboard/NavigationMenu all exhibit the issue.

gaborcsardi added a commit that referenced this issue Nov 24, 2021
Partially addresses #1236, in that there are no more check warnings,
but there still some dangling links to the super methods, which
we can probably fix.
@gaborcsardi
Copy link
Member

@bmoretz I believe that the https://github.com/r-lib/roxygen2/tree/fix/r6-private-super branch will solve the immediate issues, i.e. the check warnings should be gone.

@bmoretz
Copy link

bmoretz commented Nov 25, 2021

@gaborcsardi Thank you sir! Appreciate you taking the time to look into.

@hadley hadley added bug an unexpected problem or unintended behavior R6 6️⃣ labels Mar 29, 2022
hadley added a commit that referenced this issue Jul 10, 2022
hadley added a commit that referenced this issue Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior R6 6️⃣
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants