create_package() emits warning re: invalid ORCID id with recent r-devel

[kevinushey]

To reproduce, try running the following from a shell:

R --vanilla -s -e 'usethis::create_package("example", open = FALSE, rstudio = FALSE)'

I see:

kevin@MBP-P2MQ:/var/folders/9n/3nxsgkrj0rlfx196kzdttpww0000gn/T
$ R --vanilla -s -e 'usethis::create_package("example", open = FALSE, rstudio = FALSE)'
R(95718,0x203bccf40) malloc: nano zone abandoned due to inability to reserve vm space.
✔ Creating example/.
✔ Setting active project to "/private/var/folders/9n/3nxsgkrj0rlfx196kzdttpww0000gn/T/example".
✔ Creating R/.
✔ Writing DESCRIPTION.
Package: example
Title: What the Package Does (One Line, Title Case)
Version: 0.0.0.9000
Authors@R (parsed):
    * First Last <first.last@example.com> [aut, cre] (ORCID: YOUR-ORCID-ID)
Description: What the package does (one paragraph).
License: `use_mit_license()`, `use_gpl3_license()` or friends to
    pick a license
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.2
✔ Writing NAMESPACE.
✔ Setting active project to "<no active project>".
Warning messages:
1: In person1(given = given[[i]], family = family[[i]], middle = middle[[i]],  :
  Invalid ORCID iD: ‘YOUR-ORCID-ID’.
2: In person1(given = given[[i]], family = family[[i]], middle = middle[[i]],  :
  Invalid ORCID iD: ‘YOUR-ORCID-ID’.

Note that warning messages emitted about the invalid ORCID iD. I bumped into this with an renv test that was using create_package(); here's the associated backtrace.

Warning (test-install.R:375:3): packages embedded in the project use a project-local RemoteURL
Invalid ORCID iD: 'YOUR-ORCID-ID'.
Backtrace:
     ▆
  1. └─usethis$create_package("example", rstudio = FALSE, open = FALSE) at test-install.R:375:3
  2.   └─usethis:::proj_desc_create(name, fields, roxygen)
  3.     └─usethis:::tidy_desc(desc)
  4.       ├─base::try(desc$normalize(), silent = TRUE)
  5.       │ └─base::tryCatch(...)
  6.       │   └─base (local) tryCatchList(expr, classes, parentenv, handlers)
  7.       │     └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
  8.       │       └─base (local) doTryCatch(return(expr), name, parentenv, handler)
  9.       └─desc$normalize()
 10.         └─desc:::idesc_normalize(self, private)
 11.           └─self$reformat_fields()
 12.             └─desc:::idesc_reformat_fields(self, private)
 13.               └─desc:::idesc_str(self, private, by_field = TRUE)
 14.                 └─base::vapply(...)
 15.                   └─desc (local) FUN(X[[i]], ...)
 16.                     ├─base::format(private$data[[col]], mode = mode)
 17.                     └─desc:::format.DescriptionAuthorsAtR(private$data[[col]], mode = mode)
 18.                       └─desc:::parse_authors_at_r(x$value)
 19.                         ├─base::tryCatch(eval(parse(con, encoding = "UTF-8")), error = identity)
 20.                         │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 21.                         │   └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 22.                         │     └─base (local) doTryCatch(return(expr), name, parentenv, handler)
 23.                         ├─base::eval(parse(con, encoding = "UTF-8"))
 24.                         │ └─base::eval(parse(con, encoding = "UTF-8"))
 25.                         └─utils::person(...)
 26.                           └─base::lapply(...)
 27.                             └─utils (local) FUN(X[[i]], ...)
 28.                               ├─base::with(...)
 29.                               └─base::with.default(...)
 30.                                 └─base::eval(substitute(expr), data, enclos = parent.frame())
 31.                                   └─base::eval(substitute(expr), data, enclos = parent.frame())
 32.                                     └─utils (local) person1(...)

The warning appears to be emitted from the desc package, when desc$normalize() is called. I'm not sure whether this is better resolved in usethis or desc, though.

[jennybc]

cc @gaborcsardi

(I haven't thought about this yet, but looped you in because of the connection to desc.)

I note that usethis also puts an invalid license in as a default, which doesn't seem to cause a problem:

usethis/R/description.R

Line 128 in 2cc9e5a

License = "`use_mit_license()`, `use_gpl3_license()` or friends to pick a license",

The basic mindset is that we're scaffolding and we're fine with the user seeing a note, warning, or error the first time that they do R CMD check. That is their nudge to modify such fields (e.g. License and the ORCID in person()).

However it looks like some new behaviour in r-devel causes a warning at package creation time, which I don't want. (This wasn't clear to me from #2059, which only reported a warning at package build time.)

[hadley]

This is also causing build failures for usethis.

[hadley]

The check looks pretty robust:

https://github.com/wch/r-source/blob/be69922efa4e9c01c9169c9ba15557df813168e5/src/library/tools/R/orcidtools.R#L36-L52

So I don't see an obvious to construct an obviously fake ORCID that would still pass the test.

[jennybc]

It would be a bummer to drop the ORCID from the scaffold, because I'm convinced that will lead to less usage of the field. But we might not have a choice?

[gaborcsardi]

We can try to work around this in desc, but it is tricky, because we definitely want to catch the case when the user forgets to fill in the correct values. Ideally, the warning would show up in R CMD check only, maybe?

[gaborcsardi]

Or maybe we can use a real orcid for a fake person, e.g. https://orcid.org/0000-0002-1825-0097

But then we'd need a way to catch this ORCID when R CMD check-ing the package. Maybe we could contribute this to base R?

[jennybc]

I made a credible start on insulating usethis from this new check. But I was having to apply so much suppressWarnings() and gymnastics in the tests that I have concluded the best way forward is to drop the placeholder ORCID identifier from the default DESCRIPTION fields.

Recording a few findings here, in case it's useful in the future.

---

Putting this tidy_desc() call inside suppressWarnings() is 85% of the "solution" to dealing with the new warning:

usethis/R/proj-desc.R

Line 18 in a9afa2c

tidy_desc(desc)

But the other 15% to really finish the job is sufficiently yucky that I'm not doing it.

---

Overview of ORCID identifiers: https://support.orcid.org/hc/en-us/articles/360006897674-Structure-of-the-ORCID-Identifier#:~:text=ORCID%20iDs%20are%20assigned%20randomly,%2D0010%2D0000%2D0000.

This whitepaper explains the structure of ORCID identifiers and, at the very end, gives 3 that are fake but well-formed: https://docs.google.com/document/d/1awd6PPguRAdZsC6CKpFSSSu1dulliT8E3kHwIJ3tD5o/edit?tab=t.0#heading=h.n4gh01ccn1yv

These fake ORCIDs would be safe to use as placeholder ORCIDs, in terms of not triggering R's new warning in utils::person(). But they would be unsafe to use as a placeholder in terms of actual R package devs not realizing they are a placeholder.