Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

gp issues #319

Open
chainsawriot opened this issue Aug 30, 2023 · 3 comments
Open

gp issues #319

chainsawriot opened this issue Aug 30, 2023 · 3 comments

Comments

@chainsawriot
Copy link
Collaborator

(We can ignore the long line)

── GP rio ──────────────────────────────────────────────────────────────────────

It is good practice to

  ✖ write unit tests for all functions, and all package code in
    general. 84% of code lines are covered by test cases.

    R/compression.R:21:NA
    R/compression.R:24:NA
    R/compression.R:25:NA
    R/compression.R:47:NA
    R/compression.R:62:NA
    ... and 151 more lines

  ✖ avoid long code lines, it is bad for readability. Also,
    many people prefer editor windows that are about 80 characters
    wide. Try make your lines shorter than 80 characters

    R/arg_reconcile.R:23:81
    R/arg_reconcile.R:26:81
    R/arg_reconcile.R:51:81
    R/arg_reconcile.R:97:81
    R/characterize.R:5:81
    ... and 292 more lines

  ✖ avoid calling setwd(), it changes the global environment.
    If you need it, consider using on.exit() to restore the working
    directory.

    R/compression.R:35:13
    R/compression.R:36:5
    R/compression.R:45:5

  ✖ avoid sapply(), it is not type safe. It might return a
    vector, or a list, depending on the input data. Consider using
    vapply() instead.

    R/export_methods.R:89:20
    R/fwf2.R:46:31
    R/fwf2.R:48:17
    R/gather_attrs.R:40:13
    R/import_list.R:55:7
    ... and 3 more lines

  ✖ avoid 1:length(...), 1:nrow(...), 1:ncol(...), 1:NROW(...)
    and 1:NCOL(...) expressions. They are error prone and result 1:0 if
    the expression on the right hand side is zero. Use seq_len() or
    seq_along() instead.

    R/import_methods.R:377:21
    R/import_methods.R:443:22

@chainsawriot
Copy link
Collaborator Author

chainsawriot commented Aug 31, 2023

  • setwd
  • *apply
  • 1:

chainsawriot added a commit that referenced this issue Aug 31, 2023
chainsawriot added a commit that referenced this issue Sep 3, 2023
@chainsawriot chainsawriot reopened this Sep 4, 2023
@chainsawriot
Copy link
Collaborator Author

The setwd() in compression.R is not needed if zip is done with root

rio/R/compression.R

Lines 29 to 52 in 8bf9a23

filename <- normalizePath(filename)
tmp <- tempfile()
dir.create(tmp)
on.exit(unlink(tmp, recursive = TRUE), add = TRUE)
file.copy(from = filename, to = file.path(tmp, basename(filename)), overwrite = TRUE)
wd <- getwd()
on.exit(setwd(wd), add = TRUE)
setwd(tmp)
if (type == "zip") {
o <- zip(cfile2, files = basename(filename))
} else {
if (type == "tar") {
type <- "none"
}
o <- tar(cfile2, files = basename(filename), compression = type)
}
setwd(wd)
if (o != 0) {
stop(sprintf("File compression failed for %s!", cfile))
}
file.copy(from = file.path(tmp, cfile2), to = cfile, overwrite = TRUE)
unlink(file.path(tmp, cfile2))
return(cfile)
}

dir.create(tmp <- tempfile())
dir.create(file.path(tmp, "mydir"))
cat("first file", file = file.path(tmp, "mydir", "file1"))
cat("second file", file = file.path(tmp, "mydir", "file2"))

zipfile <- tempfile(fileext = ".zip")
zip::zip(zipfile, files = list.files(file.path(tmp, "mydir")), root = file.path(tmp, "mydir"))
zip::zip_list(zipfile)
#>   filename compressed_size uncompressed_size           timestamp permissions
#> 1    file1              15                10 2023-09-04 11:41:24         664
#> 2    file2              16                11 2023-09-04 11:41:24         664
#>      crc32 offset
#> 1 00effe3a      0
#> 2 735af9a0     66

Created on 2023-09-04 with reprex v2.0.2

@chainsawriot
Copy link
Collaborator Author

chainsawriot commented Sep 4, 2023

zip is a dependency of openxlsx anyway; and it also allows Windows support (without RTools).

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

No branches or pull requests

1 participant