Merge pull request #105 from spectral-cockpit/104-fix-timezone-parsing

- fix sample and time metadata parsing for `read_opus(dsn, data_only = TRUE)`
spectral-cockpit · Mar 12, 2024 · fb58297 · fb58297
2 parents 96c88be + 2449d67
commit fb58297
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 10 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -23,7 +23,7 @@ Description: Read data from OPUS binary files of Fourier-Transform infrared
 License: MIT + file LICENSE
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.2.3
+RoxygenNote: 7.3.1
 URL: https://github.com/spectral-cockpit/opusreader2
 BugReports: https://github.com/spectral-cockpit/opusreader2/issues
 Suggests: knitr, rmarkdown, testthat (>= 3.0.0), future.apply, future, progressr

diff --git a/R/extract_metadata.R b/R/extract_metadata.R
@@ -1,7 +1,6 @@
 get_basic_metadata <- function(ds_list) {
   timestamp <- get_meta_timestamp(ds_list)
 
-
   basic_metadata <- data.frame(
     # opus_filename,
     # opus_path,
@@ -26,7 +25,7 @@ get_meta_timestamp <- function(ds_list) {
 
   time_hour_tz <- strsplit(x = save_file_time, split = " ")[[1L]]
   time_hour <- paste(time_hour_tz[1L], time_hour_tz[2L])
-  tz <- gsub(pattern = "\\(|\\)", "", x = time_hour_tz[3L])
+  tz <- gsub(pattern = "\\(|\\)|\\s+", "", x = time_hour_tz[3L])
   etc_tz <- paste0("Etc/", tz) # see ?strptime for details
 
   # note that negative offsets denote UTC+x time stamps

diff --git a/R/parse_opus.R b/R/parse_opus.R
@@ -126,12 +126,11 @@ parse_opus <- function(raw, data_only) {
 
   dataset_list <- name_output_list(dataset_list)
 
-
   if (data_only) {
     if (any(grepl("^ab$|^refl$", names(dataset_list)))) {
       dataset_list <- extract_data(
         dataset_list,
-        c("ab", "refl", "ab_data_param", "refl_data_param")
+        c("ab", "refl", "ab_data_param", "refl_data_param", "history", "sample")
       )
     } else {
       dataset_list <- extract_data( # nolint
@@ -140,14 +139,16 @@ parse_opus <- function(raw, data_only) {
           "ab_no_atm_comp",
           "refl_no_atm_comp",
           "ab_no_atm_comp_data_param",
-          "refl_no_atm_comp_data_param"
+          "refl_no_atm_comp_data_param",
+          "history",
+          "sample"
         )
       )
     }
-  } else {
-    dataset_list <- lapply(dataset_list, calc_parameter_chunk_size)
   }
 
+  dataset_list <- lapply(dataset_list, calc_parameter_chunk_size)
+
   dataset_list <- lapply(dataset_list, function(x) parse_chunk(x, raw))
 
   data_types <- get_data_types(dataset_list) # nolint
@@ -158,7 +159,10 @@ parse_opus <- function(raw, data_only) {
   )
 
   if (data_only) {
-    dataset_list <- dataset_list[lapply(dataset_list, class) == "data"]
+    dataset_list <- dataset_list[
+      lapply(dataset_list, class) == "data" | names(dataset_list) == "history" |
+        names(dataset_list) == "sample"
+    ]
   }
 
   dataset_list <- sort_list_by(dataset_list)

diff --git a/R/read_opus.R b/R/read_opus.R
@@ -221,6 +221,10 @@ read_opus_single <- function(dsn, data_only = FALSE) {
 
   data <- c(list(basic_metadata = basic_metadata), parsed_data)
 
+  if (isTRUE(data_only)) {
+    data <- data[!names(data) %in% c("history", "sample")]
+  }
+
   attr(data, "dsn_filename") <- dsn_filename
 
   return(data)

diff --git a/inst/WORDLIST b/inst/WORDLIST
@@ -2,6 +2,7 @@ Agroecosystems
 ai
 al
 analytics
+artefacts
 aut
 automatec
 Baumann
@@ -15,6 +16,7 @@ Config
 Continous
 continous
 cph
+cran
 cre
 CSIRO
 decrypter
@@ -64,11 +66,14 @@ PARAM
 parsable
 Philipp
 PhSm
+pierreroudier
 postfixed
 pre
 precessor
 progressr
 PRs
+pypi
+qedsoftware
 Quant
 QUANT
 Raman

diff --git a/tests/testthat/test-read_opus.R b/tests/testthat/test-read_opus.R
@@ -1,3 +1,14 @@
-test_that("all test files are parsed without warning", {
+test_that("all test files are parsed without warnings and errors", {
+  expect_no_error(read_opus(dsn = "../../inst/extdata/test_data"))
   expect_no_warning(read_opus(dsn = "../../inst/extdata/test_data"))
 })
+
+test_that("all test files are parsed without warnings and errors when
+          `data_only = TRUE`", {
+  expect_no_error(
+    read_opus(dsn = "../../inst/extdata/test_data", data_only = TRUE)
+  )
+  expect_no_warning(
+    read_opus(dsn = "../../inst/extdata/test_data", data_only = TRUE)
+  )
+})