using flint package to establish better lints and fixes

Author: njtierney

.Rbuildignore

@@ -50,3 +50,7 @@ LICENSE
 ^__autograph*
 ^testlog
 ^data-raw$
+
+
+# flint files
+^flint$

R/checkers.R

@@ -38,17 +38,15 @@ check_tf_version <- function(alert = c("none",
     correct_tfp = have_tfp()
   )
 
-  if ((all(requirements_valid))) {
-
-    if (!greta_stash$python_has_been_initialised) {
+  py_not_init <- !greta_stash$python_has_been_initialised
+  requirements_valid_py_not_init <- all(requirements_valid) && py_not_init
+  if (requirements_valid_py_not_init) {
 
       cli_process_done(
         msg_done = "Initialising python and checking dependencies ... done!")
       cat("\n")
       greta_stash$python_has_been_initialised <- TRUE
 
-    }
-
   }
 
   if (!all(requirements_valid)) {
@@ -100,7 +98,7 @@ check_dims <- function(...,
 
   # as text, for printing
   dims_paste <- vapply(dim_list, paste, "", collapse = "x")
-  dims_text <- paste(dims_paste, collapse = ", ")
+  dims_text <- toString(dims_paste)
 
   # which are scalars
   scalars <- vapply(elem_list, is_scalar, FALSE)
@@ -734,7 +732,7 @@ check_dependencies_satisfied <- function(target,
 
     # build the message
     if (any(matches)) {
-      names_text <- paste(unmet_names, collapse = ", ")
+      names_text <- toString(unmet_names)
       msg <- cli::format_error(
         message = c(
           "Please provide values for the following {length(names_text)} \\
@@ -1972,7 +1970,7 @@ check_is_greta_array <- function(x,
 check_missing_infinite_values <- function(x,
                                           optional,
                                           call = rlang::caller_env()){
-  contains_missing_or_inf <- !optional & any(!is.finite(x))
+  contains_missing_or_inf <- !optional & !all(is.finite(x))
   if (contains_missing_or_inf) {
     cli::cli_abort(
       message = c(

R/distribution.R

@@ -88,19 +88,19 @@
   }
 
   # if distribution isn't scalar, make sure it has the right dimensions
-  ## TODO fix explaining variable
-  if (!is_scalar(value)) {
-    if (!identical(dim(greta_array), dim(value))) {
-      cli::cli_abort(
-        c(
-          "left and right hand sides have different dimensions. ",
-          "The distribution must have dimension of either \\
+  not_scalar <- !is_scalar(value)
+  lhs_rhs_different_dim <- !identical(dim(greta_array), dim(value))
+  not_scalar_different_dims <- not_scalar && lhs_rhs_different_dim
+  if (not_scalar_different_dims) {
+    cli::cli_abort(
+      c(
+        "left and right hand sides have different dimensions. ",
+        "The distribution must have dimension of either \\
           {.val {paste(dim(greta_array), collapse = 'x')}} or {.val 1x1},\\
           but instead has dimension \\
           {.val {paste(dim(value), collapse = 'x')}}"
-        )
       )
-    }
+    )
   }
 
   # assign the new node as the distribution's target

R/extract_replace_combine.R

@@ -100,7 +100,7 @@ NULL
   dummy_out <- do.call(.Primitive("["), call_list, envir = pf)
   rm("._dummy_in", envir = pf)
 
-  if (any(is.na(dummy_out))) {
+  if (anyNA(dummy_out)) {
     cli::cli_abort(
       "subscript out of bounds"
     )
@@ -196,7 +196,7 @@ NULL
   dummy_out <- do.call(.Primitive("["), call_list, envir = pf)
   rm("._dummy_in", envir = pf)
 
-  if (any(is.na(dummy_out))) {
+  if (anyNA(dummy_out)) {
     cli::cli_abort(
       "subscript out of bounds"
     )
@@ -211,7 +211,7 @@ NULL
         "number of items to replace is not a multiple of replacement length"
       )
     } else {
-      replacement <- rep(replacement, length.out = length(index))
+      replacement <- rep_len(replacement, length(index))
     }
   }
 
@@ -542,7 +542,7 @@ length.greta_array <- function(x) {
     dims <- c(dims, 1L)
   }
 
-  if (any(is.na(dims))) {
+  if (anyNA(dims)) {
     cli::cli_abort(
       "the dims contain missing values"
     )

R/inference.R

@@ -249,10 +249,12 @@ mcmc <- function(
     max_samplers <- future::nbrOfWorkers()
 
     # divide chains up between the workers
-    chain_assignment <- sort(rep(
-      seq_len(max_samplers),
-      length.out = chains
-    ))
+    chain_assignment <- sort(
+      rep_len(
+        seq_len(max_samplers),
+        length.out = chains
+    )
+    )
 
     # divide the initial values between them
     initial_values_split <- split(initial_values, chain_assignment)

R/inference_class.R

@@ -914,7 +914,6 @@ hmc_sampler <- R6Class(
       return(
         sampler_kernel
       )
-      # nolint end
     },
     sampler_parameter_values = function() {
 
@@ -999,7 +998,6 @@ rwmh_sampler <- R6Class(
       return(
         sampler_kernel
       )
-      # nolint end
     },
     sampler_parameter_values = function() {
       epsilon <- self$parameters$epsilon
@@ -1044,7 +1042,6 @@ slice_sampler <- R6Class(
       return(
         sampler_kernel
       )
-      # nolint end
     },
     sampler_parameter_values = function() {
       max_doublings <- as.integer(self$parameters$max_doublings)

R/progress_bar.R

@@ -24,7 +24,7 @@ create_progress_bar <- function(phase, iter, pb_update, width, ...) {
   # pad the frmat so that the width iterations counter is the same for both
   # warmup and sampling
   digit_diff <- nchar(max(iter)) - nchar(iter_this)
-  count_pad <- paste0(rep(" ", 2 * digit_diff), collapse = "")
+  count_pad <- paste(rep(" ", 2 * digit_diff), collapse = "")
 
   # formatting
   format_text <- glue::glue(
@@ -73,7 +73,7 @@ iterate_progress_bar <- function(pb, it, rejects, chains, file = NULL) {
       }
       # pad the end of the line to keep the update bar a consistent width
       pad_char <- pmax(0, 2 - nchar(reject_perc_string))
-      pad <- paste0(rep(" ", pad_char), collapse = "")
+      pad <- paste(rep(" ", pad_char), collapse = "")
 
       reject_text <- glue::glue("| {reject_perc_string}% bad{pad}")
     } else {

R/unknowns_class.R

@@ -56,7 +56,7 @@ print.unknowns <- function(x, ..., n = 10) {
 }
 
 # create an unknowns array from some dimensions
-unknowns <- function(dims = c(1, 1), data = as.numeric(NA)) {
+unknowns <- function(dims = c(1, 1), data = NA_real_) {
   x <- array(data = data, dim = dims)
   as.unknowns(x)
 }

R/utils.R

@@ -991,8 +991,10 @@ is_using_cpu <- function(x){
 `%||%` <- function(x, y) if (is.null(x)) y else x
 
 message_if_using_gpu <- function(compute_options){
-  if (is_using_gpu(compute_options)) {
-    if (getOption("greta_gpu_message") %||% TRUE){
+  gpu_used <- is_using_gpu(compute_options)
+  greta_gpu_message <- getOption("greta_gpu_message") %||% TRUE
+  gpu_used_and_message <- gpu_used && greta_gpu_message
+  if (gpu_used_and_message) {
       cli::cli_inform(
         c(
           "NOTE: When using GPU, the random number seed may not always be \\
@@ -1003,7 +1005,6 @@ message_if_using_gpu <- function(compute_options){
           "{.code options(greta_gpu_message = FALSE)}"
         )
       )
-    }
   }
 }
 
@@ -1169,7 +1170,7 @@ pretty_dim <- function(x){
   x_dim <- dim(x)
   print_dim_x <- x_dim %||% x
 
-  prettied_dim <- paste0(print_dim_x, collapse = "x")
+  prettied_dim <- paste(print_dim_x, collapse = "x")
   prettied_dim
 }
 

flint/cache_file_state.rds

@@ -0,0 +1,51 @@
+keep:
+  - any_duplicated
+  - any_is_na
+  - class_equals
+  - condition_message
+  - double_assignment
+  - duplicate_argument
+  - empty_assignment
+  - equal_assignment
+  - equals_na
+  - expect_comparison
+  - expect_identical
+  - expect_length
+  - expect_named
+  - expect_not
+  - expect_null
+  - expect_true_false
+  - expect_type
+  - for_loop_index
+  - function_return
+  - implicit_assignment
+  - is_numeric
+  - length_levels
+  - length_test
+  - lengths
+  - library_call
+  - list_comparison
+  - literal_coercion
+  - matrix_apply
+  - missing_argument
+  - nested_ifelse
+  - numeric_leading_zero
+  - outer_negation
+  - package_hooks
+  - paste
+  - redundant_equals
+  - redundant_ifelse
+  - rep_len
+  - right_assignment
+  - sample_int
+  - semicolon
+  - seq
+  - sort
+  - stopifnot_all
+  - T_and_F_symbol
+  - todo_comment
+  - undesirable_function
+  - undesirable_operator
+  - unnecessary_nesting
+  - unreachable_code
+  - which_grepl

flint/config.yml

@@ -0,0 +1,97 @@
+id: true_false_symbol
+language: r
+severity: warning
+rule:
+  pattern: T
+  kind: identifier
+  not:
+    any:
+      - precedes:
+          any:
+            - pattern: <-
+            - pattern: =
+            - regex: ^~$
+      - follows:
+          any:
+            - pattern: $
+            - regex: ^~$
+      - inside:
+          any:
+            - kind: parameter
+            - kind: call
+            - kind: binary_operator
+              follows:
+                regex: ^~$
+                stopBy: end
+          stopBy:
+            kind:
+              argument
+fix: TRUE
+message: Use TRUE instead of the symbol T.
+
+---
+
+id: true_false_symbol-2
+language: r
+severity: warning
+rule:
+  pattern: F
+  kind: identifier
+  not:
+    any:
+      - precedes:
+          any:
+            - pattern: <-
+            - pattern: =
+            - regex: ^~$
+      - follows:
+          any:
+            - pattern: $
+            - regex: ^~$
+      - inside:
+          any:
+            - kind: parameter
+            - kind: call
+            - kind: binary_operator
+              follows:
+                regex: ^~$
+                stopBy: end
+          stopBy:
+            kind:
+              argument
+fix: FALSE
+message: Use FALSE instead of the symbol F.
+
+---
+
+id: true_false_symbol-3
+language: r
+severity: warning
+rule:
+  pattern: T
+  kind: identifier
+  precedes:
+    any:
+      - pattern: <-
+      - pattern: =
+  not:
+    inside:
+      kind: argument
+message: Don't use T as a variable name, as it can break code relying on T being TRUE.
+
+---
+
+id: true_false_symbol-4
+language: r
+severity: warning
+rule:
+  pattern: F
+  kind: identifier
+  precedes:
+    any:
+      - pattern: <-
+      - pattern: =
+  not:
+    inside:
+      kind: argument
+message: Don't use F as a variable name, as it can break code relying on F being FALSE.

flint/rules/builtin/T_and_F_symbol.yml

@@ -0,0 +1,13 @@
+# id: absolute_path-1
+# language: r
+# severity: warning
+# rule:
+#   kind: string_content
+#   any:
+#     - regex: '^~[[:alpha:]]'
+#     - regex: '^~/[[:alpha:]]'
+#     - regex: '^[[:alpha:]]:'
+#     - regex: '^(/|~)$'
+#     - regex: '^/[[:alpha:]]'
+#     - regex: '^\\'
+# message: Do not use absolute paths.