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

conflicting note on global variable assignment #1969

Closed
krishnakeshav opened this issue May 26, 2023 · 8 comments
Closed

conflicting note on global variable assignment #1969

krishnakeshav opened this issue May 26, 2023 · 8 comments

Comments

@krishnakeshav
Copy link

lintr gives conflicting notes for scenario below -

library(utils)
utils::globalVariables(c("g_var"))

test_gvar <- function() {
  g_var <- 5
}
print(g_var)

image
It's likely for variable to be used outside the function when I have already declared it to be global with utils::globalVariables(c("g_var"))
It should have been resolved as per this.

@MichaelChirico
Copy link
Collaborator

is this part of a script or a package?

@krishnakeshav
Copy link
Author

krishnakeshav commented May 26, 2023

@MichaelChirico

is this part of a script or a package?

Issue originally occurred in a package, but the above example is a script for the sake of reproducing.
The easiest way around this is to add return(g_var).

@AshesITR
Copy link
Collaborator

Your code doesn't do what you think it does, and the warning is justified.

If you want to overwrite g_var in the outer scope from within the function, you need to use <<-.

@krishnakeshav
Copy link
Author

Your code doesn't do what you think it does, and the warning is justified.

If you want to overwrite g_var in the outer scope from within the function, you need to use <<-.

using <<- in conjunction to glovalVariables also gives warning, see below -
image
It seems to me that <<- is not recognized as an operator to lint.

@AshesITR
Copy link
Collaborator

Sorry for being unclear.
You should of course also define the global variable before writing to it.

@krishnakeshav
Copy link
Author

krishnakeshav commented May 29, 2023

Sorry for being unclear. You should of course also define the global variable before writing to it.

Thanks @AshesITR , it works well with scripts. In package, when I run lint active package from R studio, I don't get any notes. However, upon running R CMD check, I get the note as below -

no visible binding for '<<-' assignment to ‘result_index_list’

Note that, I have different code in case of R script in package.

result_index_list <<- NULL
some_fun <- function() {
  result_index_list <<- some_list_assignment
}

In above, if I use result_index_list <- NULL, then I get following runtime error -

Error in FUN(X[[i]], ...) :
cannot change value of locked binding for 'result_index_list'

I have R/globals.R with utils::globalVariables(c("result_index_list"))
Does lint behave differently in R CMD check than the scripts ?

@AshesITR
Copy link
Collaborator

AshesITR commented May 29, 2023

@krishnakeshav I recommend you read the R Packages section on internal state for packages to see how you should define and use global state in Packages.

Here is a MWE copied from there:

the <- new.env(parent = emptyenv())
the$favorite_letters <- letters[1:3]

#' Report my favorite letters
#' @export
mfl2 <- function() {
  the$favorite_letters
}

#' Change my favorite letters
#' @export
set_mfl2 <- function(l = letters[24:26]) {
  old <- the$favorite_letters
  the$favorite_letters <- l
  invisible(old)
}

For plain scripts, the error "cannot change value of locked binding ..." will disappear.

@krishnakeshav
Copy link
Author

@krishnakeshav I recommend you read the R Packages section on internal state for packages to see how you should define and use global state in Packages.

Here is a MWE copied from there:

the <- new.env(parent = emptyenv())
the$favorite_letters <- letters[1:3]

#' Report my favorite letters
#' @export
mfl2 <- function() {
  the$favorite_letters
}

#' Change my favorite letters
#' @export
set_mfl2 <- function(l = letters[24:26]) {
  old <- the$favorite_letters
  the$favorite_letters <- l
  invisible(old)
}

For plain scripts, the error "cannot change value of locked binding ..." will disappear.

This worked for me. Thanks again @AshesITR

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

3 participants