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

How to fix "Rd warning missing file link" when linking to an alias #707

Closed
rabutler opened this issue Jan 31, 2018 · 29 comments
Closed

How to fix "Rd warning missing file link" when linking to an alias #707

rabutler opened this issue Jan 31, 2018 · 29 comments
Labels
feature a feature request or enhancement markdown ⬇️

Comments

@rabutler
Copy link

Currently, if you link to a function in another package that has an alias that points to a different Rd/html file, you end up with a missing file link warning when building the package.

For example, if your documentation includes:

#' Here is a link to \code{\link[dplyr:transmute]{dplyr::transmute()}}
#' Or the markdown equivalent: [dplry::transmute()]

You end up getting the following warning when building the package:

...
*** installing help indices
  converting help for package 'simpleTest'
    finding HTML links ...    hello                                   html  
Rd warning: C:/user/RPackages/simpleTest/man/hello.Rd:11: missing file link 'transmute'
 done
...

As was pointed out on Stack Overflow, you can resolve this by linking to the file, instead of the alias:

#' This link works with no warning: \code{\link[dplyr-mutate]{dplyr::transmute()}}

However, I don't think you can do this if using the markdown version. It also seems slightly dangerous to link to a RD/html file, which seems more likely to change than a function name; at least it seems possible that a developer might change the Rd file names without thinking of the issues, while function names are at least deprecated first.

So, would it make sense to search the .libPaths() using tools::findHTMLlinks(level = 2), while roxygen2 is converting the Roxygen comments to Rd files, and add in the appropriate file link for any functions that uses aliases to point to a different file name?

I'm not sure how much extra overhead this would add, but it would make reading the build messages a little nicer.

@hadley hadley added feature a feature request or enhancement markdown ⬇️ labels Jun 28, 2018
wibeasley pushed a commit to OuhscBbmc/REDCapR that referenced this issue Jul 10, 2018
Markup was used to link to help documentation for base::package_version() in @details and utils::packageVersion() in @return, but those functions do not have individual help files. Instead, they link to the common help files for base::numeric_version() and utils::packageDescription() respectively. Because the markup approach does not allow to point to the common help files ala r-lib/roxygen2#707, the markup was replaced with corresponding roxygen formatting commands. This patch will remove the error messages for missing files during package installation.
@xhdong-umd
Copy link

This issue seemed still exist, it's also reported for other users after the supposed fix.

Installing package in Mac don't have the warning, but installing same package in windows has it. The actual link in help is working in both case. I'm not sure why there is difference in platforms.

jmgirard added a commit to jmgirard/usethis that referenced this issue Apr 25, 2019
jmgirard added a commit to jmgirard/usethis that referenced this issue Apr 25, 2019
pkimes added a commit to areyesq89/SummarizedBenchmark that referenced this issue Jun 22, 2019
jennybc pushed a commit to r-lib/usethis that referenced this issue Jul 3, 2019
jennybc pushed a commit to r-lib/usethis that referenced this issue Jul 3, 2019
* Link to topic file rather than alias

This change will prevent Rd warnings during "Install and Restart" on Windows. However, it may make the link more fragile.
r-lib/roxygen2#707
https://stackoverflow.com/questions/48430093/how-do-i-resolve-rd-warning-missing-file-link-when-building-packages-in-rstudi/48478698#48478698

* Manually restore fixes from this PR

* http --> https

* Add a NEWS bullet
@jennybc
Copy link
Member

jennybc commented Jul 3, 2019

I confirm that this problem (the warnings at install time) appears to be Windows-specific. So when this gets tackled, that platform is the main one to test on. I changed the affected usethis templates to link to Rd files, not topics, which cleared the warnings during installation on Windows.

Most relevant section of WRE is 2.5 Cross-references, especially this paragraph (although I beg to differ re "rarely needed"):

There are two other forms of optional argument specified as \link[pkg]{foo} and \link[pkg:bar]{foo} to link to the package pkg, to files foo.html and bar.html respectively. These are rarely needed, perhaps to refer to not-yet-installed packages (but there the HTML help system will resolve the link at run time) or in the normally undesirable event that more than one package offers help on a topic (in which case the present package has precedence so this is only needed to refer to other packages). They are currently only used in HTML help (and ignored for hyperlinks in LaTeX conversions of help pages), and link to the file rather than the topic (since there is no way to know which topics are in which files in an uninstalled package). The only reason to use these forms for base and recommended packages is to force a reference to a package that might be further down the search path. Because they have been frequently misused, the HTML help system looks for topic foo in package pkg if it does not find file foo.html.

@gaborcsardi
Copy link
Member

Is this really causing any problems? I have packages on CRAN that produce this warning, but seemingly CRAN is fine with this.

@jennybc
Copy link
Member

jennybc commented Jul 4, 2019

No, I don't think this causes problems. I see lots of packages emitting tons of these warnings when installing on Windows.

I think silencing the installation warnings is just good from a "not crying wolf" POV. And the wording of WRE somehow gives me a sense that CRAN could decide to care about this one day, since they refer to it as "misuse".

@gaborcsardi
Copy link
Member

gaborcsardi commented Jul 4, 2019

normally undesirable event that more than one package offers help on a topic

Well, I think that while this is undesirable, it is perfectly normal. This is what namespaces are for. Having to link to the name of the file is not ideal at all. You would want to be able to link to an exported function or to a topic, that's the right level of abstraction. That is also independent of how the help topics are organized into files.

So personally, I would not worry about this, until it really becomes an issue.

@jennybc
Copy link
Member

jennybc commented Jul 4, 2019

Fair enough. I agree that you really should be able to link to another package's function or topic and be insulated from how that maintainer chooses to document it.

@ghost
Copy link

ghost commented Jul 24, 2019

FWIW, I also get this with rstudio server on an RHEL AWS box. I can't easily reprex, 'cuz I'm linking from one internal company package to another, but the links are like this:
\code{\link[MMRdb]{generate_sql_where}}

And that help is actually in @rdname generate_sql

It isn't really an issue, it just makes scary noise when I document. The one thing I worry about is that it'll mask an actual error at some point (because I'm used to seeing a bunch of output there).

@rabutler
Copy link
Author

It seems like this is bugging more folks than me, so I have put together a potential solution at https://github.com/rabutler/roxygen.

It modifies parse_link() to search for the html link for a given function/object that is returned by tools::findHTMLlinks(). Then, it replaces the html page name with the page name found in this search if the page is from the user specified package. This ensures you won't link to stats::filter() since it is found in tools::findHTMLlinks() when you were trying to link to dplyr::filter().

This does come at some expense. After this addition, using microbenchmark and an examle taken from the tests (included at the end), the median execution time increased from 37 ns to 272 ns. I think this is likely because it currently calls tools::findHTMLlinks() every time that a link is parsed. There's probably a better way to do this so that it isn't called every time parse_link() is called.

If this concept seems reasonable and useful, then I'll spend some more time to improve the efficiency. Additionally, I have a few questions that would be good to get the package authors to weigh in on:

  1. Currently, if there is no html file found, then the current behavior is replicated, i.e., the markdown is translated to Rd format based on provided package and function name. Is that acceptable?
  2. The code I provided would not catch links to files that have a base R function name with the same name. Ex: tools::findHTMLlinks() removes duplicates, so if you search it for filter() it will return the page for stats::filter() and not dplyr::filter(). This does not really matter for dplyr::filter() because it is documented in filter.Rd/html. However, if it was documented in some other file, then this would not fix the original warnings. I'm not sure of any examples that meet this criteria, or if it is worth diving into a solution to catch these cases too. Any thoughts here would be appreciated.

Code to check evaluation time:

microbenchmark::microbenchmark(
  roc_proc_text(roc, "
      #' Title
      #'
      #' Description, see [`name`][dest],
      #' [`function`][function()],
      #' [`filter`][stats::filter()],
      #' [`bar`][pkg::bar],
      #' [`terms`][terms.object],
      #' [`abc`][abc-class].
      #' @md
      foo <- function() {}")[[1]]
  , times = 100)

@gaborcsardi
Copy link
Member

First of all, this is not a roxygen issue. If you write your documentation without roxygen, in Rd files, then you still get the warnings.

Second, you already have an option to link to the file, as above:

#' This link works with no warning: \code{\link[dplyr-mutate]{dplyr::transmute()}}

Third, I think linking to the file is not right. You should not rely on or care about which file documents certain functions and topics. Should that change, your documentation links will be broken. If you link to a function name, that will work, as long as that function is still exported from the linked package.

I understand that the warning is annoying. I think we should try to fix this in base R, simply by avoiding the warning.

@gaborcsardi
Copy link
Member

@rabutler This said, my third point is subjective, and getting this fixed in base R might not be simple, so if you open a PR, it might get merged, still. :) So thanks for investigating this!

@rabutler
Copy link
Author

rabutler commented Aug 2, 2019

Thanks @gaborcsardi I appreciate the thoughts.

I understand it's not an roxygen issue, but to your last comment, thought it might be easier to address in roxygen than base R, especially since roxygen is parsing these files already.

I think my proposed fix actually prevents the user from having to link to the file. If you use \code{\link[dplyr-mutate]{dplyr::transmute()}}, the dplyr documentation could change and so this would no longer work. (If transmute() got it's own page, then this would point you to the wrong topic.)

However, if you stick with the markdown version [dplyr::transmute()], the user never has to change this. As long as they have dplyr installed (I think that's a fair assumption if they are developing a package that links to it), then the page name will be found when findHTMLLinks() is called. If the other package's documentation changes, the user will never be the wiser, and the link will still work.

Either way, thanks for considering. I'll clean up a bit more, and try to not call findHTMLLinks() for every call to parse_link() and submit a PR.

@gaborcsardi
Copy link
Member

However, if you stick with the markdown version [dplyr::transmute()], the user never has to change this.

The installed version of the linking package will break with the new dplyr version, and it would need to be reinstalled.

@rabutler
Copy link
Author

rabutler commented Aug 2, 2019

Wouldn't that also be the case if you were using \code{\link[dplyr-mutate]{dplyr::transmute()}} if the page name got changed?

@gaborcsardi
Copy link
Member

Sure. That's why just linking to the function name is better.

@vincentarelbundock
Copy link

@gaborcsardi not sure what you mean by "it is only used for new submissions", but a new version of my already-on-CRAN package was just bounced with this on the CRAN Debian pretest (it passes without warning on their Windows machine):

* checking Rd cross-references ... WARNING
Non-file package-anchored link(s) in documentation object 'reexports.Rd':
  ‘[magrittr]{%>%}’

@gaborcsardi
Copy link
Member

They do not use it in their daily checks, and they'll not archive packages because of this. But they'll enforce it for newly submitted versions, new packages included.

mjskay added a commit to mjskay/tidybayes that referenced this issue Jun 14, 2020
muschellij2 added a commit to muschellij2/iglu that referenced this issue Jun 16, 2020
dfsp-spirit added a commit to dfsp-spirit/fsbrain that referenced this issue Jun 18, 2020
dfsp-spirit added a commit to dfsp-spirit/fsbrain that referenced this issue Jun 18, 2020
chacalle added a commit to ihmeuw-demographics/demCore that referenced this issue Jun 23, 2020
dchiu911 added a commit to AlineTalhouk/diceR that referenced this issue Jun 27, 2020
TomKellyGenetics added a commit to TomKellyGenetics/graphsim that referenced this issue Jul 1, 2020
TomKellyGenetics added a commit to TomKellyGenetics/graphsim that referenced this issue Jul 9, 2020
chilampoon added a commit to reactome/ReactomeContentService4R that referenced this issue Oct 28, 2020
@chilampoon
Copy link

chilampoon commented Oct 28, 2020

\code{\link[dplyr-mutate]{dplyr::transmute()}} this one cannot link to the help page, though it won't have warnings.

\code{\link[dplyr:mutate]{dplyr::transmute()}} this one works well and also no warnings.

@gaborcsardi
Copy link
Member

gaborcsardi commented Oct 28, 2020

@chilampoon The file name has changed in dplyr.

Also, linking to topics/files has changed in R itself as well, so what works with one R version might not work with another.

@chilampoon
Copy link

chilampoon commented Oct 28, 2020

@chilampoon The file name has changed in dplyr.

Also, linking to topics/files has changed in R itself as well, so what works with one R version might not work with another.

Okay, I didn't use the dplyr function to test actually, but just found that [pkg-pageName] didn't work on my end with R 4.0.3.

@gaborcsardi
Copy link
Member

Actually, the file name hasn't changed, the dash must have been a typo.

@gaborcsardi
Copy link
Member

But also, with R-devel and recent roxygen you should not need to do that. That's why this issue is closed.

@chilampoon
Copy link

chilampoon commented Oct 28, 2020

@gaborcsardi I got a Rd warning in a Windows platform check for my package, something like Rd warning: ... file link 'function' in package 'pkg' does not exist and so has been treated as a topic

I found that the 'function' is documented with several other functions in a single help page, that's why \code{\link[pkg]{function}} triggered the warning. If I used \code{\link[pkg:pageName]{pkg::function()}} then it was OK

@gaborcsardi
Copy link
Member

If you write \code{\link[pkg]{function}} that's not a roxygen issue. Roxygen does not do anything with those links. This issue is about the [dplyr::transmute()] style roxygen links.

@chilampoon
Copy link

If you write \code{\link[pkg]{function}} that's not a roxygen issue. Roxygen does not do anything with those links. This issue is about the [dplyr::transmute()] style roxygen links.

Yea, writing the [dplyr::transmute()] in Roxygen comments then it's fine!

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 markdown ⬇️
Projects
None yet
Development

No branches or pull requests

9 participants