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.

Sign up for GitHub

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

Jump to bottom

create_format_terra_vect: temp fix for issue with replacing function body and {covr} #39

Closed
wants to merge 1 commit into from
Closed

create_format_terra_vect: temp fix for issue with replacing function body and {covr} #39

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions R/tar-terra-vect.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ create_format_terra_vect <- function(filetype, options, ...) {
terra::writeVector(
object,
path,
filetype = NULL,
filetype = geotargets::geotargets_option_get("gdal.vector.driver"),
overwrite = TRUE,
options = NULL
options = geotargets::geotargets_option_get("gdal.vector.creation_options")
)
}
body(.write_terra_vector)[[2]][["filetype"]] <- filetype
Copy link
Owner

@njtierney njtierney Mar 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we even actually need to modify the function body directly? I'm fairly sure those parts will be evaluated appropriately?

E.g.,

example_funfun <- function(sep_type){
  .write_fun <- function(object, file){
    # loudly declare the quote type when writing
    readr::write_lines(x = object,
                       file = file,
                       sep = sep_type)
    cat("Hi! The separation marker is: '", sep_type,"'", sep = "")
    cat("\n")
    cat("We wrote the file out to:\n", file, sep = "")
  }
  
  return(
    list(
      read = readr::read_csv,
      write = .write_fun
    )
  )
  
}

my_example <- example_funfun(",")

# sep_type isn't evaluated yet
my_example$write
#> function(object, file){
#>     # loudly declare the quote type when writing
#>     readr::write_lines(x = object,
#>                        file = file,
#>                        sep = sep_type)
#>     cat("Hi! The separation marker is: '", sep_type,"'", sep = "")
#>     cat("\n")
#>     cat("We wrote the file out to:\n", file, sep = "")
#>   }
#> <environment: 0x12a2669e0>

# but sep_type gets evaluated later when the function is used:
my_example$write(
  object = "1,2,3,4", 
  file = tempfile(fileext = ".csv")
  )
#> Hi! The separation marker is: ','
#> We wrote the file out to:
#> /var/folders/9c/k3wqmhhx4qsb3fd66n4prhlw0000gq/T//RtmpUdvv1a/file104a0120aafb7.csv

Created on 2024-03-16 with reprex v2.1.0

Session info 
sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value
#>  version  R version 4.3.3 (2024-02-29)
#>  os       macOS Sonoma 14.3.1
#>  system   aarch64, darwin20
#>  ui       X11
#>  language (EN)
#>  collate  en_US.UTF-8
#>  ctype    en_US.UTF-8
#>  tz       Australia/Hobart
#>  date     2024-03-16
#>  pandoc   3.1.1 @ /Applications/RStudio.app/Contents/Resources/app/quarto/bin/tools/ (via rmarkdown)
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package     * version date (UTC) lib source
#>  bit           4.0.5   2022-11-15 [1] CRAN (R 4.3.0)
#>  bit64         4.0.5   2020-08-30 [1] CRAN (R 4.3.0)
#>  cli           3.6.2   2023-12-11 [1] CRAN (R 4.3.1)
#>  crayon        1.5.2   2022-09-29 [1] CRAN (R 4.3.0)
#>  digest        0.6.34  2024-01-11 [1] CRAN (R 4.3.1)
#>  evaluate      0.23    2023-11-01 [1] CRAN (R 4.3.1)
#>  fansi         1.0.6   2023-12-08 [1] CRAN (R 4.3.1)
#>  fastmap       1.1.1   2023-02-24 [1] CRAN (R 4.3.0)
#>  fs            1.6.3   2023-07-20 [1] CRAN (R 4.3.0)
#>  glue          1.7.0   2024-01-09 [1] CRAN (R 4.3.1)
#>  hms           1.1.3   2023-03-21 [1] CRAN (R 4.3.0)
#>  htmltools     0.5.7   2023-11-03 [1] CRAN (R 4.3.1)
#>  knitr         1.45    2023-10-30 [1] CRAN (R 4.3.1)
#>  lifecycle     1.0.4   2023-11-07 [1] CRAN (R 4.3.1)
#>  magrittr      2.0.3   2022-03-30 [1] CRAN (R 4.3.0)
#>  pillar        1.9.0   2023-03-22 [1] CRAN (R 4.3.0)
#>  pkgconfig     2.0.3   2019-09-22 [1] CRAN (R 4.3.0)
#>  purrr         1.0.2   2023-08-10 [1] CRAN (R 4.3.0)
#>  R.cache       0.16.0  2022-07-21 [2] CRAN (R 4.3.0)
#>  R.methodsS3   1.8.2   2022-06-13 [2] CRAN (R 4.3.0)
#>  R.oo          1.26.0  2024-01-24 [2] CRAN (R 4.3.1)
#>  R.utils       2.12.3  2023-11-18 [2] CRAN (R 4.3.1)
#>  R6            2.5.1   2021-08-19 [1] CRAN (R 4.3.0)
#>  readr         2.1.5   2024-01-10 [1] CRAN (R 4.3.1)
#>  reprex        2.1.0   2024-01-11 [2] CRAN (R 4.3.1)
#>  rlang         1.1.3   2024-01-10 [1] CRAN (R 4.3.1)
#>  rmarkdown     2.25    2023-09-18 [1] CRAN (R 4.3.1)
#>  rstudioapi    0.15.0  2023-07-07 [1] CRAN (R 4.3.0)
#>  sessioninfo   1.2.2   2021-12-06 [2] CRAN (R 4.3.0)
#>  styler        1.10.2  2023-08-29 [2] CRAN (R 4.3.0)
#>  tibble        3.2.1   2023-03-20 [1] CRAN (R 4.3.0)
#>  tidyselect    1.2.0   2022-10-10 [1] CRAN (R 4.3.0)
#>  tzdb          0.4.0   2023-05-12 [1] CRAN (R 4.3.0)
#>  utf8          1.2.4   2023-10-22 [1] CRAN (R 4.3.1)
#>  vctrs         0.6.5   2023-12-01 [1] CRAN (R 4.3.1)
#>  vroom         1.6.5   2023-12-05 [1] CRAN (R 4.3.1)
#>  withr         3.0.0   2024-01-16 [1] CRAN (R 4.3.1)
#>  xfun          0.42    2024-02-08 [1] CRAN (R 4.3.1)
#>  yaml          2.3.8   2023-12-11 [1] CRAN (R 4.3.1)
#> 
#>  [1] /Users/nick/Library/R/arm64/4.3/library
#>  [2] /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library
#> 
#> ──────────────────────────────────────────────────────────────────────────────

Or is there some deeper stuff happening with targets here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that will work, or at least I tried it before arriving at the body<- solution I proposed in #11 here. Tried doing it again now because I hoped I had missed something.

What you propose works fine interactively in your global environment, or in a reprex, etc. but the context where those tar_format() functions get evaluated you will not have the e.g. sep_type object defined.

Essentially the functions tar_format() uses need to be self-contained, not relying on the prior parent frame. You actually want sep_type to be evaluated sooner as it is when we modify the function body.

If you implement your suggestion for tar_terra_rast() you will get something like this:

Loading required namespace: terra
▶ dispatched target test_terra_rast
✖ errored target test_terra_rast
✖ errored pipeline [0.13 seconds]
Error:
! Error running targets::tar_make()
Error messages: targets::tar_meta(fields = error, complete_only = TRUE)
Debugging guide: https://books.ropensci.org/targets/debugging.html
How to ask for help: https://books.ropensci.org/targets/help.html
Last error message:
    _store_ object 'filetype' not found
Last error traceback:
    No traceback available.

# tar-terra-rast.R: create_format_terra_raster
#...
 .myfun <- function(filetype, gdal) {
        .write_terra_raster <- function(object, path) {
            terra::writeRaster(
                object,
                path,
                filetype = filetype,
                overwrite = TRUE,
                gdal = gdal, 
            )
        }
        return(list(write = .write_terra_raster))
    }

    targets::tar_format(
        read = function(path) terra::rast(path),
        write = .myfun(filetype, gdal)$write,
        marshal = function(object) terra::wrap(object),
        unmarshal = function(object) terra::unwrap(object)
    )
# ...

Some other ideas I tried were using purrr::partial() to compose functions with some arguments pre-specified and couldn't get that working. Also forcing evaluation and injecting values with {rlang} metaprogramming operators also did not appear to work, but that might be worth re-investigating now.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, thanks for explaining that! It looks like we lose all reference to the parent frames here, I guess it might be evaluated in a separate R process, so making changes directly to the body of the function makes sense. I'm surprised that metaprogramming things from {rlang} don't work out, it feels like this is the sort of thing where you want to change some aspect of the AST or something. But effectively, I think, that's what you're doing with the clever body()<- calls.

If it continues to be a problem we can re-evaluate some possible solutions with rlang - I might even tag in Will into the issues to see if he has had this problem?

Expand Down