Merck / pkglite

Compact Package Representations
https://merck.github.io/pkglite/
GNU General Public License v3.0
30 stars 4 forks source link

seg_char and any seq(...) - dependent function #41

Open SHAESEN2 opened 1 year ago

SHAESEN2 commented 1 year ago

It would be helpful to include checks before sending code into a seq(...) function. In my case I had an empty file in a folder and got an error while running the pack function:

Error in seq.default(from = 1L, to = nchars, by = nmax) :
wrong sign in 'by' argument

I traced it all the way to

seg_char <- function(x, nmax) {
    nchars <- length(x)
    pos <- seq(from = 1L, to = nchars, by = nmax)
    short <- nchars <= nmax
    nlines <- if (short) 1L else length(pos)
    pos_start <- if (short) 1L else pos[seq_len(nlines)]
    pos_end <- if (short) nchars else c(pos[2L:nlines] - 1L, nchars)
    lapply(seq_len(nlines), function(i) x[pos_start[i]:pos_end[i]])
  }

Since the binary file had 0 bytes and no content the seq(from = 1L, to = 0, by = 64) gave the error. I propose to built in a safety net to check if files have content (eg remove empty files in a sanitize function, eg if nchars>1 then do ... else give informative error to the user to investigate)

nanxstats commented 1 year ago

Nice catch! Thanks so much for finding this edge case. Mostly it sounds like a relatively simple fix - perhaps keeping the empty file is still desirable. I will probably patch it in the next few months and update the progress here.