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

roxygen2 reported error after upgrade from 6.1.1 to 7.0.2 #1008

Closed
genomaths opened this issue Dec 13, 2019 · 13 comments · Fixed by #1009
Closed

roxygen2 reported error after upgrade from 6.1.1 to 7.0.2 #1008

genomaths opened this issue Dec 13, 2019 · 13 comments · Fixed by #1009
Labels
bug rd ✍️

Comments

@genomaths
Copy link

@genomaths genomaths commented Dec 13, 2019

Hi there
I have been using roxygen2 6.1.1 for all my R packages. Now after upgrade roxygen2 to version 7.0.2, I got the following error:

Error in if (self$has_section(type) && !overwrite) { : missing value where TRUE/FALSE needed

I reinstalled the old version 6.1.1 to keep updating the projects.
Since I focus my attention on the maths and not in the documentation, perhaps I am missing some important detail.

You can check the project https://github.com/genomaths/MethylIT/tree/MethylIT_0.3.2_beta (roxygen2 6.1.1)

Would you please give me some tip on how to solve this problem?

Best,
Robersy

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

Hi, this is because you have

#' @description NULL

here: https://github.com/genomaths/MethylIT/blob/ddec3eb401b31820ffd31dd9eb469781611da3f7/R/ggamma.R#L74 and also elsewhere. roxygen2 should give a better error message though.

What are you trying to achieve by setting the description tag to NULL?

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

Actually if there is another @description tag, then roxygen2 should just merge them and not error. Let me fix this.

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

Here is a small reprex:

roxygen2::roc_proc_text(roxygen2::rd_roclet(), "
  #' Title
  #'
  #' @description desc1
  #' @description NULL
  foo <- function() NULL
  ")[[1]]
#> Error in if (self$has_section(type) && !overwrite) {: missing value where TRUE/FALSE needed

Created on 2019-12-13 by the reprex package (v0.3.0)

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

But actually, even this fails:

roxygen2::roc_proc_text(roxygen2::rd_roclet(), "
  #' Title
  #'
  #' @description NULL
  foo <- function() NULL
  ")[[1]]
#> Error in if (self$has_section(type) && !overwrite) {: missing value where TRUE/FALSE needed

Created on 2019-12-13 by the reprex package (v0.3.0)

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

@hadley it seems that having NULL for @description or @details will trigger this error. What should roxygen2 do if either of these are NULL?

  • not generate that section at all? This will be an R CMD check error I guess, but if that's what the user wants, let them have it.
  • generate a default \description{} section, as if the topic had no (explicit or implicit) description at all? I.e. use the title? (\details{} is not compulsory AFAIR, so that should be fine.)
  • give an error that there \description section is empty and that's not allowed?

@hadley
Copy link
Member

@hadley hadley commented Dec 13, 2019

I think warn and drop the tag so you get the default behaviour?

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

Yeah, that makes sense. I'll submit a PR.

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

OK, so there is actually a case when suppressing \description{} makes sense, for package level manual pages. From WRE:

This is mandatory except for package-overview files.

So for these we should not warn, and should not drop either, I guess, because @description NULL is good for suppressing the auto-generated section?

@hadley
Copy link
Member

@hadley hadley commented Dec 13, 2019

@gaborcsardi supporting that seems relatively low priority to me.

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

@hadley Right, I agree. OTOH we already have an exception for re-exports pages, and supporting it is a one line change.

gaborcsardi added a commit that referenced this issue Dec 13, 2019
Now they are ignored, except for `@description NULL` in
package level documentation, where it can be used to suppress the
auto-generated Description section

Closes #1008.
@genomaths
Copy link
Author

@genomaths genomaths commented Dec 13, 2019

Hi guys

Thank you very much for your answers. I really appreciate it.

In the case of ggamma probability distribution function, where I just write down the four statistical functions dggamma, pggamma, rggamma, and qggamma in the same file, it does not makes sense to me the repetition of the content for \description and \details sections four times.

I mean, if I must do it to prevent the error, then I will do it. However, as @gaborcsardi suggests, if you can introduce the flexibility to go with the option NULL or something similar, then that will be great. This is the application of "Minimum writing code principle", the code version of "Ockham's razor principle" :-)

Please let me know if @gaborcsardi pull request went through.

Best,
Robersy

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Dec 13, 2019

it does not makes sense to me the repetition of the content for \description and \details sections four times.

roxygen2 will not repeat a section just because the page documents multiple functions. You can just remove the @description NULL and @details NULL lines.

@genomaths
Copy link
Author

@genomaths genomaths commented Dec 13, 2019

I do remember when I wrote the first version of the ggamma function (using old version of roxygen2), I was forced to introduce these sections and to write NULL solved the problem. Perhaps, I did something wrong at that time that force me to do it. Anyway, I will follow your suggestion.

Thank you very much for your support

@hadley hadley added bug rd ✍️ labels Mar 5, 2020
gaborcsardi added a commit that referenced this issue Mar 6, 2020
Now they are ignored, except for `@description NULL` in
package level documentation, where it can be used to suppress the
auto-generated Description section

Closes #1008.
gaborcsardi added a commit that referenced this issue Mar 6, 2020
Now they are ignored, except for `@description NULL` in
package level documentation, where it can be used to suppress the
auto-generated Description section

Closes #1008.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug rd ✍️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants