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

Documentation not found for reexports #330

Open
owenjonesuob opened this issue Jul 24, 2023 · 4 comments
Open

Documentation not found for reexports #330

owenjonesuob opened this issue Jul 24, 2023 · 4 comments

Comments

@owenjonesuob
Copy link

Error description

If a module imports, and then re-exports, an object from another module, then box::help() doesn't look for documentation in the right place.

We can see this with ./bio from the "Nesting modules" section of the "Getting started" vignette. If we implement bio using the "reexport" pattern at the end of that section:

#' @export
box::use(./seq[...])

Then we fail to retrieve help for is_valid(), because box::help() is looking for documentation in bio rather than in seq:

box::use(./bio)
box::help(bio$is_valid)
#> Error in box::help(bio$is_valid) : 
#>  no documentation available for “is_valid” in module “./bio”

R version

4.3.1

‘box’ version

1.1.3 (from CRAN), build/HEAD (from GitHub)

@klmr
Copy link
Owner

klmr commented Jul 25, 2023

Thanks for the report, I can confirm that this is a bug. Unfortunately I am not sure that it will be easy to fix, since the way reexports are currently handled loses all information about them being reexports.

(Note to self:)

I’ll probably have to change the return type of parse_export_specs to include information about where a given name is reexported from (if it is a reexport), maybe via an attribute. Then I’ll need to add that information to the namespace information in load_from_source. And then I’ll need to adapt box::help to select the correct namespace.

@owenjonesuob
Copy link
Author

I'm afraid I only managed to dig a little way into this myself before getting stuck - I will share my only finding in case it is helpful, though I suspect you might have a better way internally of getting the same info!

With the same example from above:

attr(attr(bio$is_valid, "srcref"), "srcfile")
#> /path/to/box/vignettes/bio/seq.r

From some quick ad hoc testing, this seems to point to the right file (i.e. the file where the function is defined) even if we re-export a function all the way up a stack of modules.

@klmr
Copy link
Owner

klmr commented Aug 6, 2023

Hmm. This is actually more complex than I thought.

Consider the following module a.r:

#' @export
box::use(./other_mod[alias = name])

If a user now imports a and runs box::help(a$alias), they want to be shown the help of name from a/other_module.r. But that help document will use name instead of alias everywhere, which would confuse the user (consider reading the documentation for the function foo$bar() and bar is never mentioned, but instead the documentation mentions how to use frob).

Not sure what a good solution for this is, except to force users to provide new documentation for documents that are re-exported under an alias. And annotating reeexports with ‘roxygen2’ isn’t supported yet (and there are issues with that as well; such as enforcing that documentation is applied to a reexport that names a single object).

@klmr
Copy link
Owner

klmr commented Aug 20, 2024

(Note to self:) also asked about on Stack Overflow.

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

No branches or pull requests

2 participants