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

Tag suggestion: @globals #461

Closed
thomasp85 opened this issue Mar 10, 2016 · 8 comments
Closed

Tag suggestion: @globals #461

thomasp85 opened this issue Mar 10, 2016 · 8 comments

Comments

@thomasp85
Copy link
Member

With the increase in use of non-standard evaluation, it would be fantastic if it was possible in roxygen comments to indicate which NSE names are used in the function and have roxygen automatically compile the globalVariables() call into a new file. I know that often there are SE versions of the functions, that are better used in package development, but this is not always the case (data.table springs to mind).

I suggest the addition of the @Globals tag with a straightforward syntax:

#' @globals variable1 variable2 function1

which would result in:

globalVariables(
  names = c(
    'variable1',
    'variable2',
    'function1'
  )
)

being written to r/roxygen-globals.r (or some other name - just a file that roxygen "owns")

@hadley
Copy link
Member

hadley commented Aug 29, 2016

I think in the long-term this is likely to be fixed in other ways, so I'm hesitant to add support for the work around at this time.

@hadley hadley closed this as completed Aug 29, 2016
@MichaelChirico
Copy link
Contributor

This would indeed help users importing data.table in particular... the symbols we define here are quite common for importers to hit issues with:

https://github.com/Rdatatable/data.table/blob/a5537dcbbf0711936167fe1e63111653bc7ac803/R/data.table.R#L18-L24

.SD = .N = .I = .GRP = .BY = .EACHI = NULL

We're in the process (Rdatatable/data.table#3076) of trying to get packages that Depend on data.table to move to Importing and this is coming up a lot. We have a section in our Importing data.table vignette on just this but something automated from roxygen2 would be convenient.

in the long-term this is likely to be fixed in other ways

@hadley any update on this or on your thinking here?

@hadley
Copy link
Member

hadley commented Sep 22, 2019

You’re thinking that this would generate a globalVariables() definition?

(I was hoping that R CMD check would eventually recognise function attribute that would flag arguments as using NSE, but I don’t think there’s been any movement. It might be possible to contribute a patch to codetools — but I don’t think that would help with data table since the NSE mostly happens in a method)

@gaborcsardi
Copy link
Member

gaborcsardi commented Sep 22, 2019

Roxygen does not (so far) generate any code, it does not write any of the files in the R directory, so this would be a new thing.

Also, I am not sure if roxygen simplifies this issue, frankly, unless it is possible to somehow solve it in NAMESPACE, which we already generate. But AFAIK this is not possible.

There are various ways to solve the problem without roxygen, e.g. in your package, you can provide a function, that the users can just call, within their package, at install time, i.e. outside of their regular functions, and this function could call globalVariables(). This makes your package a build time dependency, but I think that's OK in this case.

@MichaelChirico
Copy link
Contributor

thanks for the response Gabor.

this would be a new thing do do

A good point. I'll leave it at that here.

It would still be convenient to offer users some sort of guidance here, perhaps as part of rcmdcheck?

@hadley
Copy link
Member

hadley commented Sep 23, 2019

@MichaelChirico for the data.table case, why not recommend that users import those symbols? (like usethis::use_tidy_eval())

@MichaelChirico
Copy link
Contributor

MichaelChirico commented Sep 23, 2019

Good idea... have filed Rdatatable/data.table#3903

Would you be open to adding usethis::use_data_table()? I would file a PR.

@hadley
Copy link
Member

hadley commented Sep 23, 2019

Yes, definitely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants