dev_guide icon indicating copy to clipboard operation
dev_guide copied to clipboard

Recommend / mandate that all defaults are clearly documented

Open Bisaloo opened this issue 3 years ago • 7 comments

In many cases, the docs will read:

#' @param test A logical value determining if ...

I think it would be much better practice and clearer for the user if this was:

#' @ param test A logical value (default `TRUE`) determining if ...

I don't have strong feelings on the exact syntax here. The default could be indicated at the start of the end of the documentation string, it could be inside parentheses or in the main text, etc. But I believe it is really important that it is written somewhere.

Bisaloo avatar Apr 12 '22 09:04 Bisaloo

Good idea @Bisaloo, and I do agree ... but any recommendations regarding this are also going to subject to quite a bit of change over the next year or two, primarily because autotest will slowly come to be "recommended" for all new submissions, and that package will ensure issues like this, and a lot more, are "automatically" addressed prior to submission. That means we could add something along those lines to the Dev Guide now, but then it would likely only be removed later. I think quite a number of issues like these will be discussed more extensively in the context of discussions about how to transition autotest from a helpful tool to something that is recommended, and utlimately to something that is required. Not that that helps you for now, but important to keep in mind nevertheless.

mpadge avatar Apr 12 '22 11:04 mpadge

It makes a lot sense and I'm looking forward to seeing autotest in more places.

However:

  • I don't see a downside to have it both in autotest and in the documentation here. In other words, I'm not sure I understand why you say "then it would likely only be removed later".
  • I think a non-negligible portion of readers (myself included) use the devguide as some kind of reference for the current community-approved best practices. They might not want/be able to go through rOpenSci review (and therefore possibly not use autotest) but still want to follow these best practices.

Bisaloo avatar Apr 12 '22 11:04 Bisaloo

Good points!

Would there be a way for that part to be added automatically based on the code?

I.e. if I have

bla <- function(a = TRUE) {
  # stuff
}

could the default for a be added without adding it manually to the roxygen2 markup?

maelle avatar Apr 19 '22 12:04 maelle

Not easily, alas, because there's no way to auto - insert descriptions in previously written descriptions. The closest would be to insert at very start in parentheses. Things like that might happen down the line in autotest but not yet.

mpadge avatar Apr 20 '22 06:04 mpadge

Good points!

Would there be a way for that part to be added automatically based on the code?

I.e. if I have

bla <- function(a = TRUE) {
  # stuff
}

could the default for a be added without adding it manually to the roxygen2 markup?

That would be good from a DRY perspective but in theory, there is no guarantee that the value provided there is really the default. See for example this (admittedly very bad but still valid) code:

test <- function(a = 1) {
  
  if (missing(a)) {
    a <- 2
  }
  
  return(a)
}

test()
#> [1] 2

Created on 2022-04-20 by the reprex package (v2.0.1.9000)

Bisaloo avatar Apr 20 '22 08:04 Bisaloo

@Bisaloo would you like to make a PR to the dev guide to recommend the documentation of default values?

maelle avatar Jul 04 '22 14:07 maelle

@Bisaloo a PR is still welcome if / when you have time. :smile_cat:

maelle avatar Jul 21 '22 08:07 maelle