isoverse / isoreader

Read IRMS (Isotope Ratio Mass Spectrometry) data files into R
http://isoreader.isoverse.org
GNU General Public License v2.0
8 stars 6 forks source link

consider implementing iso_file_lists as vctr classes #78

Open sebkopf opened 5 years ago

sebkopf commented 5 years ago

the new vctrs package provides efficient implementations of complex type vectors and would solve several issues with S3 method dispatch for different types of iso files objects (e.g. continuous flow vs. dual inlet).

the caveat is that individual iso_file objects are not as easily clearly defined to stay compatible with different file format imports.

gotta think this through carefully whether it makes sense

japhir commented 3 years ago

I think this would be great. If I understand correctly it would get rid of these lines, right?

https://github.com/isoverse/isoreader/blob/13886c5f7067e516c7daedfa34b981a7997bf14b/R/aggregate_data.R#L395

I think it would be best if you have a length-0 iso_files list, it should return the tibble with all the desired columns of the correct type. I'm not sure if this is right, but for one of my functions I'm now considering:

  # part of the `get_inits()` function, function should always return below data structure?
  if (nrow(.did) == 0L) {
    return(
      tibble(file_id = character(), Analysis = character(),
             s44_init = double(), r44_init = double())
    )
  }

but it seems like vctrs might do this kind of housekeeping in a better way?

japhir commented 3 years ago

or is this what you use https://github.com/isoverse/isoreader/blob/bba86e6342e17965e83f00937a85d686b8c7485d/R/aggregate_data.R#L826 for?