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

Building docs requires 'Authors@R' field #727

Closed
kenahoo opened this Issue Jun 7, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@kenahoo
Copy link

kenahoo commented Jun 7, 2018

I'm trying to build pkgdown docs for several different packages that I don't necessarily control. When I do so, I often get this error:

> pkgdown::build_site(pkg='.../mypackage', examples=FALSE, new_process = FALSE)
══ Building pkgdown site ══════════════════════════════════════════════════════════════════════════════════════
Reading from: '.../mypackage'
Writing to:   '.../mypackage/docs'
── Initialising site ──────────────────────────────────────────────────────────────────────────────────────────
── Building home ──────────────────────────────────────────────────────────────────────────────────────────────
 Hide Traceback
 
 Rerun with Debug
 Error in ensure_authors_at_r(self) : No 'Authors@R' field!
You can create one with $add_author 

The Authors@R field isn't required, so it would be nice to support building docs when it's not present, i.e. when the DESCRIPTION file just has Author and Maintainer fields.

(Note that I also had to do new_process=FALSE because using the latest Git master, if I don't supply that argument, I get:

Error in pkgdown::build_site(...) : 
  unused arguments (document = TRUE, new_process = FALSE)
Calls: withCallingHandlers ... <Anonymous> -> <Anonymous> -> .handleSimpleError -> h
Execution halted

.)

@jayhesselberth jayhesselberth added the bug label Jun 7, 2018

@jayhesselberth

This comment has been minimized.

Copy link
Collaborator

jayhesselberth commented Jun 7, 2018

Could you please provide a link to a package with a DESCRIPTION with Author / Maintainer fields that fails during pkgdown::build_site()?

The build_site(new_process=TRUE) error is a separate issue (please file one issue per issue). What is your version of callr?

@jayhesselberth

This comment has been minimized.

Copy link
Collaborator

jayhesselberth commented Jun 7, 2018

Dup of #357.

That error encourages use of Authors@R, though it's not informative (i.e., what provides $add_author?). Not sure if we want to support Author: and Maintainer: fields, but I understand that their use in other packages is out of your control.

@jayhesselberth

This comment has been minimized.

Copy link
Collaborator

jayhesselberth commented Jun 7, 2018

Also see r-lib/desc#44.

@jayhesselberth

This comment has been minimized.

Copy link
Collaborator

jayhesselberth commented Jun 7, 2018

There is an unmerged PR to do this on the desc side (r-lib/desc#46). Probably better that it happen there then in pkgdown.

@kenahoo

This comment has been minimized.

Copy link
Author

kenahoo commented Jun 7, 2018

Agreed, this would be good on the desc side.

Hadley's response so far has been to encourage people to convert to Authors@R, and I also encourage that, but sometimes it's out of my control, and since Author is supported in DESCRIPTION I do think it should be supported in pkgdown.

@kenahoo kenahoo referenced this issue Jun 7, 2018

Closed

Only one Author #44

@kenahoo

This comment has been minimized.

Copy link
Author

kenahoo commented Jun 7, 2018

Here's an example, I'll also add this at r-lib/desc#44.

x <- "Package: example
Type: Package
Title: Example DESC
Version: 0.0.1
Author: John Doe
Maintainer: John Doe <jdoe@example.com>
Description: Here is a description.
License: GPL-2
"

d <- description$new(text=x)
d$get_authors()    # Fails

# Correct the issue & retry
d$add_author(d$get_maintainer())
d$get_authors()    # Succeeds
@kenahoo

This comment has been minimized.

Copy link
Author

kenahoo commented Jun 7, 2018

The following patch would allow processing to continue when there's a Maintainer field but no Authors@R field.

diff --git a/R/build-home-authors.R b/R/build-home-authors.R
index e3cf890..d5519b2 100644
--- a/R/build-home-authors.R
+++ b/R/build-home-authors.R
@@ -20,7 +20,11 @@ data_authors <- function(pkg = ".") {
 }
 
 pkg_authors <- function(pkg, role = NULL) {
-  authors <- unclass(pkg$desc$get_authors())
+  authors <-
+    if (pkg$desc$has_fields('Authors@R'))
+      unclass(pkg$desc$get_authors())
+    else
+      list(given=pkg$desc$get_maintainer())  # Don't bother to parse
 
   if (is.null(role)) {
     authors

This is assuming such functionality doesn't get handled in the desc package, where it's probably better.

@muschellij2

This comment has been minimized.

Copy link

muschellij2 commented Jun 7, 2018

To be clear, the PR on r-lib/desc#46 will not "fix" the problem, as there will still be an error. It adds the coerce_authors_at_r() function added to the desc object to make an Authors@R field.

@jayhesselberth

This comment has been minimized.

Copy link
Collaborator

jayhesselberth commented Jun 9, 2018

list(given=pkg$desc$get_maintainer()) isn't good enough as it doesn't parse John Doe <john.doe@example.com> into the components (given, family, email) that pkgdown expects.

Once r-lib/desc#46 is merged, then it should be a simple check on the pkgdown side to call coerce_authors_at_r (or whatever the name ends up being) if Authors@R is not found in the DESCRIPTION.

@kenahoo

This comment has been minimized.

Copy link
Author

kenahoo commented Jun 10, 2018

Yes, I discovered that approach didn't work so well for a couple other reasons too.

Relying on coerce_authors_at_r sounds great, thanks.

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