gesistsa / rio

🐟 A Swiss-Army Knife for Data I/O
http://gesistsa.github.io/rio/
602 stars 76 forks source link

gp issues #319

Open chainsawriot opened 1 year ago

chainsawriot commented 1 year ago

(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 commented 1 year ago
chainsawriot commented 1 year ago

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

https://github.com/gesistsa/rio/blob/8bf9a23aa17722d2cd92436e08aea0c706bcedfa/R/compression.R#L29-L52

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 commented 1 year ago

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