Add use_author() #833

avalcarcel9 · 2019-07-11T14:43:18Z

I talked with @jennybc at useR! about adding functionality to easily add or adapt authors in the Authors@R field in the description. Essentially the same functionality and use as use_package(). I think expert users add their information to the .Rprofile but more novice developers still add manually and often miss the field in use_description(). This could be useful for adding an author after package development as well.

I've never contributed to a tidyverse package so I'm not sure of your conventions. I tried to look up a few similar functions in the package and mimic those styles. I am happy to adapt the function or documentation to match your conventions, in the event of bugs, or to add functionality. Additionally, I did not provide tests for the new function. If you'd like to incorporate the use_author() function/feature I'm again happy to add tests or more documentation.

Possible TODOs:
TODO allow generic authors input like desc_set_authors(authors, file = ".", normalize = FALSE) for multiple author input
TODO create addin to prompt for author information
TODO create a snippet with author information for DESCRIPTION
TODO add tests
TODO check if the author input has partial overlap with one already listed and ask user if they would like to add the person as a new author or replace the existing author

avalcarcel9 · 2019-07-11T14:52:03Z

Bug in line 89 (sorry). role must be a string in the desc::desc_add_author function. I can fix and re-pull if you're interested in incorporating.

jennybc

Thanks! Looking good. I've given some comments to move things along. You can keep committing to your add_use_author branch, push that into your fork and this PR will automatically update.

NAMESPACE

R/authors.R

jennybc · 2019-07-20T16:07:20Z

R/authors.R

+  # Adapted from use_dependency code and tools provided in the desc package
+  # TODO allow generic authors input like desc_set_authors(authors, file = ".", normalize = FALSE) for multiple author input
+  # TODO create addin to prompt for author information
+  # TODO create a snippet with author information for DESCRIPTION


These are interesting ideas, but let's set them aside at this time.

I've adjusted these TODOs to 'long term' to keep them as ideas but allow us to move forward now. I've added a few more ideas/TODOs in this version that may still be relevant in the near future.

jennybc · 2019-07-20T16:07:29Z

R/authors.R

+  # TODO allow generic authors input like desc_set_authors(authors, file = ".", normalize = FALSE) for multiple author input
+  # TODO create addin to prompt for author information
+  # TODO create a snippet with author information for DESCRIPTION
+  # TODO add tests


Definitely yes!

I added tests! I have one test I'd like to add to check that a message is returned from the use_author() function when the usethis default author (First, Last...) is in the description. I used ui_yeah() to prompt the message. It seems this is interactive and so expect_message() didn't work. Is there a way to add a test in that case?

jennybc · 2019-07-20T16:08:28Z

R/authors.R

+  # TODO create addin to prompt for author information
+  # TODO create a snippet with author information for DESCRIPTION
+  # TODO add tests
+


There should be some checks for obviously bad input here. The internal helper is_string() will be useful to you.

I didn't manually check inputs since the inputs for my function are all passed to desc package functions. They do seems to have checks on inputs when someone is input incorrectly as a vector or numeric for example. I'm happy to add these checks if you prefer they be done within the usethis function directly.

jennybc · 2019-07-20T16:12:05Z

R/authors.R

+  author = utils::person(given = given, family = given, role = role, email = email, comment = comment)
+
+  # Obtain the current DESCRIPTION fields
+  current_desc_fields <- desc::desc_fields()


Are you sure you can't just load up the DESCRIPTION file, grab the current authors (possibly none, but that's going to be very very rare), and just call desc::desc_add_author()? What I'm really saying is: does there need to be so much conditional logic?

In general, I'm comfortable assuming that we start with our own output, i.e. that we (usethis) created the DESCRIPTION file. Or that it's not pathological in some way.

I tried to incorporate this flow into this version of the function and get rid of the conditional logic. I still have some if() to assess what the state of the current description is in order to add an author or error the function.

I now error if the DESCRIPTION has 'Author:' rather than 'Authors@R' and send the user a message. I allow for addition of an author so long as 'Authors@R@' is a field (blank or pre-filled). I do not make changes if the author being input with use_author() is already in the DESCRIPTION. I ask the user if they would like to delete the usethis default author (First, Last, ...) if it is found in the DESCRIPTION.

I'd like to eventually change the error from finding 'Author' to prompting the user if they want to replace it with 'Authors@R' and continue adding and also prompt the user to add 'Authors@R' if it is missing and Authors was not found. Not sure if I'm over-thinking this though. I tried to stick with your advice and start by assuming the user used usethis or at least the generic DESCRIPTION generated when using File > New Directory > R Package...

R/authors.R

avalcarcel9 · 2019-07-22T15:15:27Z

Thanks so much @jennybc ! I'll take a look at all of this and start working on incorporating the changes this week.

…conditional if else. had to keep some for checks but got rid of major one.

…f author input

… testing. returned description to original. while playing with use_author some stuff got messed up. just copied and pasted from r-lib github.

avalcarcel9 · 2019-08-16T00:02:19Z

@jennybc This version seems to be working fine. I made sure to fetch and merge updates between my old branch and the current devel usethis on gh. Some checks on travis are failing for the R devel version but they seem to be unrelated to this content. I tried to rigorously test out the function manually myself by assessing different test cases and prompting errors. I eventually made as many of these real tests so hopefully most major bugs are out but a grad student only has so much time in the day.

Looking forward to your feedback and I hope its ready to merge!

GitHunter0 · 2020-11-01T20:24:59Z

Hey @avalcarcel9 , it is a helpful feature, any update here?

avalcarcel9 · 2020-11-01T22:41:15Z

@GitHunter0 no updates! I was waiting to hear back about next steps to get this incorporated into the package.

GitHunter0 · 2020-11-02T02:50:33Z

It is a pity, @avalcarcel9 , so let's wait for @jennybc

jennybc · 2020-11-02T15:15:00Z

I took a look recently and decided it needs work before it can be merged. For starters, copy editing of the docs and code restyling. And, much more vaguely, editing to be more consistent with existing code base.

avalcarcel9 · 2020-11-02T18:43:19Z

@jennybc is there anything I can do to help with the edits and todos or does the remaining work need to be done with someone on your team? As a sort of independent contributor I'm not sure what you mean exactly by these comments and would need some more specific TODOs to help out. But I'm still more than happy to help in any way I can!

jennybc · 2020-11-02T18:55:24Z

The contributing guide for this repo contains good information, such as re: code style:

https://github.com/r-lib/usethis/blob/master/.github/CONTRIBUTING.md

You could also read the documentation carefully and copy edit it. I think there are some missing words / typos in places.

jennybc · 2020-11-02T18:57:24Z

Also the tests use a helper that has since been renamed to create_local_package().

You generally need to merge in current main development from master of the main repo.

All of this is stuff I can do, but this is why finishing the PR isn't a quick and easy process.

avalcarcel9 · 2020-11-05T23:44:59Z

@jennybc I completely understand the process takes a long time to rigorously validate and check. I started this process while in grad school and have since moved onto a new job during this crazy time. I'd still love the opportunity to help and as time permits would love to help out with these last tasks to get the code and functionality ready to integrate. That being said, I'm not sure when that time will pop up with my new responsibilities during this crazy time. So if you get to this and I haven't contributed feel free to finish it off. If not, hopefully at some point (soon-ish) I can go through the documentation for contributing, make the changes, and re-push changes. So sorry I can't help more immediately.

Use prevailing convention for name of R/*.R file and test file

jennybc · 2023-04-21T01:57:10Z

I refactored this a lot since various things have changed considerably since this story arc first started!

jennybc · 2023-04-21T01:58:15Z

R/author.R

+#' file, creating that field if necessary. It will not modify, e.g., the role(s)
+#' or email of an existing author (judged using their "Given Family" name). For


I decided to address the 80% use case and keep things fairly simple. For more fiddly changes, I think folks should just edit DESCRIPTION or use desc themselves.

I think that's a good decision.

R/author.R

jennybc · 2023-04-21T02:03:26Z

R/author.R

+  })
+  if (any(m)) {
+    aut_name <- glue("{given %||% ''} {family %||% ''}")
+    usethis_abort(c(


usethis's UI needs an overhaul (pivoting towards cli), which I'm not necessarily biting off right now. But when we touch errors, we're phasing in usethis_abort().

For the overhaul, will the ui_oops(), ui_info() etc. be part of that? I.e., usethis_abort() is just the start?

Yeah it will be everything. Some of those function will certainly live on, but their guts will be modified to use cli.

ateucher

I think it looks great! I agree with the the 80% use case otherwise it would get overly complex. I had just a couple of suggestions/questions. Thanks for the explanations of conventions and development direction.

ateucher · 2023-04-21T18:39:00Z

R/author.R

+#' )
+#' }
+#'
+use_author <- function(given = NULL, family = NULL, ..., role = "ctb") {


Do you think we should expose the email argument explicitly? I think the others are fine behind ... but I would think email will be used the vast majority of the time.

I thought about it and did take a look at author info for some existing CRAN packages. It feels pretty common to only include email for cre and I'm not sure there's a real purpose for including email for other contributors? Anyhow, that's how I ended up here.

So I guess I'm assuming that use_author() is going to be used mostly for adding folks in a non-cre role, i.e. that the maintainer is probably already set up. And that it's fine to omit email in this case.

This did inspire me to add #' @inheritDotParams utils::person, which leads to much better documentation of ..., so that is a big improvement.

That all makes sense to me!

I worked on the examples a bit, so use of email and role is demonstrated, but I think most calls will be more like the last one: use_author("Charlie", "Brown").

Yeah, I like that

ateucher · 2023-04-21T18:45:51Z

R/author.R

+#' cre] (YOUR-ORCID-ID)` and `use_author()` offers to remove it in interactive
+#' sessions.
+#'
+#' @inheritParams utils::person


Suggested change

#' @inheritParams utils::person

#' @inheritParams utils::person

#' @param ... Arguments passed on to [utils::person()]

I think we need this since utils::person() doesn't have a ... argument, so it shows up in the use_authors.Rd file as "currently not used".

Ah, I handled this above before I got to this point in the review.

ateucher · 2023-04-21T18:46:45Z

R/author.R

+#' file, creating that field if necessary. It will not modify, e.g., the role(s)
+#' or email of an existing author (judged using their "Given Family" name). For


I think that's a good decision.

ateucher · 2023-04-21T18:51:26Z

R/author.R

+#' `use_author()` also surfaces two other situations you might want to address:
+
+#' * Explicit use of the fields `Author` or `Maintainer`. We recommend switching
+#' to the more modern `Authors@R` field instead, because it offers richer


Is it worth linking to https://r-pkgs.org/description.html#sec-description-authors-at-r here?

I'm not 100% convinced it's a net win to have lots of links from actual packages into the book, especially links to specific sections. It feels kind of brittle/perishable. So my current posture is that there has to be a really reason to do this. I did go back and read that passage and intentionally paraphrased it.

Yup, that makes sense

R/author.R

ateucher · 2023-04-21T19:00:08Z

R/author.R

+  })
+  if (any(m)) {
+    aut_name <- glue("{given %||% ''} {family %||% ''}")
+    usethis_abort(c(


For the overhaul, will the ui_oops(), ui_info() etc. be part of that? I.e., usethis_abort() is just the start?

ateucher · 2023-04-21T19:06:29Z

R/author.R

+  authors <- d$get_authors()
+  authors_given <- purrr::map(authors, "given")
+  authors_family <- purrr::map(authors, "family")
+  m <- purrr::map2_lgl(authors_given, authors_family, function(x, y) {


Do you think checking the email would also be good? It's a pretty unique identifier. I can see cases like "Andy Teucher" vs "Andrew Teucher" or "Jenny Bryan" vs "Jennifer Bryan" slipping through...

Could be (using pmap()):

identical(x, given) && identical(y, family) || identical(z, email)

But maybe that's overkill

Yeah, I convinced myself that this level of matching was overkill.

ateucher · 2023-04-21T19:26:21Z

R/author.R

+  withr::with_options(
+    list(usethis.description = NULL),
+    defaults <- use_description_defaults()
+  )


Would it be an option to pull this out into a function that returns that list, and consult it for the default Authors@R?

Good idea. Done.

ateucher · 2023-04-21T19:28:04Z

R/author.R

+    {ui_field('Authors@R')} appears to include a placeholder author:
+    {format(default_author, style = 'text')}")
+  if (any(m) && is_interactive() && ui_yeah("Would you like to remove it?")) {
+    # TODO: Do I want to suppress this output?


I think the output is fine, I think it's nice to see confirmation that it's doing something

avalcarcel9 added 2 commits July 11, 2019 10:19

added use_author. need to check

8c62373

fixing notes

fec0254

jennybc reviewed Jul 20, 2019

View reviewed changes

avalcarcel9 added 9 commits August 14, 2019 11:34

changes before merging and pulling new usethis from gh

afefd10

remove @importFrom fields and simply depend on ::

418e39a

compiled after previous changes. also used styler to adapt code style.

59da1c0

updated some todos to be more long term

e036d51

updated function code based on jennybc comments. tried to remove the …

5e2569b

…conditional if else. had to keep some for checks but got rid of major one.

Minor changes to use_author function related to ui_* constructions.

2b2ef5e

debugged and made sure conditionals worked properly in each setting o…

952c71d

…f author input

added tests for use_author. minor tweaks to function use_author while…

f313e4a

… testing. returned description to original. while playing with use_author some stuff got messed up. just copied and pasted from r-lib github.

added tests.

8679a9b

jennybc added 7 commits April 20, 2023 16:16

Merge branch 'main' into avalcarcel9-add_use_author

56200df

This file should not exist

6e5ead5

Since is singular use_author(), do same in filenames

d201f69

Use prevailing convention for name of R/*.R file and test file

Re-factor use_author()

f341550

Add to reference index

cd6e9f0

Skip this snapshot test on older R versions

bde9a67

Put the skip in the right test

7087b7e

jennybc requested a review from ateucher April 21, 2023 01:54

Mention correct function

f8a184a

jennybc reviewed Apr 21, 2023

View reviewed changes

jennybc added 2 commits April 20, 2023 19:06

Add NEWS bullet

a708caa

Polishing

634dafa

jennybc force-pushed the add_use_author branch from 56db833 to 634dafa Compare April 21, 2023 14:25

The R version rolled overnight!

263af86

ateucher approved these changes Apr 21, 2023

View reviewed changes

jennybc added 5 commits April 21, 2023 14:37

Document ... properly

92a232b

Put usethis's defaults for DESCRIPTION into a helper

fb02965

Update example

71d26cf

Fix logic

b2a2078

Merge branch 'main' into add_use_author

3f97476

jennybc changed the title ~~Add use author~~ Add use_author() Apr 21, 2023

jennybc merged commit 98f1853 into r-lib:main Apr 21, 2023

		#' file, creating that field if necessary. It will not modify, e.g., the role(s)
		#' or email of an existing author (judged using their "Given Family" name). For

	#' @inheritParams utils::person
	#' @inheritParams utils::person
	#' @param ... Arguments passed on to [utils::person()]

Add use_author() #833

Add use_author() #833

Uh oh!

Conversation

avalcarcel9 commented Jul 11, 2019

Uh oh!

avalcarcel9 commented Jul 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jennybc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avalcarcel9 Aug 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avalcarcel9 Aug 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avalcarcel9 Aug 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

avalcarcel9 commented Jul 22, 2019

Uh oh!

avalcarcel9 commented Aug 16, 2019

Uh oh!

GitHunter0 commented Nov 1, 2020

Uh oh!

avalcarcel9 commented Nov 1, 2020

Uh oh!

GitHunter0 commented Nov 2, 2020

Uh oh!

jennybc commented Nov 2, 2020

Uh oh!

avalcarcel9 commented Nov 2, 2020

Uh oh!

jennybc commented Nov 2, 2020

Uh oh!

jennybc commented Nov 2, 2020

Uh oh!

avalcarcel9 commented Nov 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jennybc commented Apr 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ateucher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

avalcarcel9 commented Jul 11, 2019 •

edited

Loading

avalcarcel9 Aug 15, 2019 •

edited

Loading

avalcarcel9 Aug 15, 2019 •

edited

Loading

avalcarcel9 Aug 15, 2019 •

edited

Loading

avalcarcel9 commented Nov 5, 2020 •

edited

Loading

ateucher Apr 21, 2023 •

edited

Loading