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.

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

read_Daybreak2R() may not properly recognise a binary file #135

Closed
mcol opened this issue Aug 12, 2024 · 3 comments
Closed

read_Daybreak2R() may not properly recognise a binary file #135

mcol opened this issue Aug 12, 2024 · 3 comments
Labels
bug (potentially) Is seems like a bug, but it's unclear what real effect it may have

Comments

@mcol
Copy link
Contributor

mcol commented Aug 12, 2024

If read_Daybreak2R() is called on a file with no .DAT extension, the following code is reached:

file2read <- suppressWarnings(readLines(file))

##check whether this is a binary file
if(!all(charToRaw(file2read[1]) <= as.raw(127))){
  stop("[read_Daybreak2R()] The provided file is no ASCII-file and cannot be imported!", call. = FALSE)
}

However, this code is not enough to prevent an attempt to read a proper binary file (as opposed to a text file with non-ASCII characters). For example, this crashes:

read_Daybreak2R(file = system.file("extdata/BINfile_V8.binx", package = "Luminescence"))
[read_Daybreak] file extension not of type '.DAT' try to import ASCII-file ... 
Error in records.row_number[x]:length(file2read) : NA/NaN argument

A possible mitigation is to call readLines(file, ok = FALSE), which raise an error directly when reading in the file if its length is not what is expected, which happens for the .binx file used above.

@mcol mcol added the bug (potentially) Is seems like a bug, but it's unclear what real effect it may have label Aug 12, 2024
@RLumSK
Copy link
Member

RLumSK commented Aug 12, 2024

@mcol Thanks! I do agree, please create a PR.

@mcol
Copy link
Contributor Author

mcol commented Aug 14, 2024

It turns out that using readLines(file, ok = FALSE) is too restrictive and would reject also good input files, but I've found a different approach that fixes the error and makes the existing tests pass.

@RLumSK
Copy link
Member

RLumSK commented Aug 14, 2024

Closed with PR#140, thanks!

@RLumSK RLumSK closed this as completed Aug 14, 2024
@mcol mcol added this to the v0.9.25 (autumn CRAN release) milestone Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug (potentially) Is seems like a bug, but it's unclear what real effect it may have
Projects
None yet
Development

No branches or pull requests

2 participants