International-Soil-Radiocarbon-Database / ISRaD

Repository for the development and release of ISRaD data and tools
https://international-soil-radiocarbon-database.github.io/ISRaD/
24 stars 15 forks source link

QAQC first pass #233

Closed bpbond closed 4 years ago

bpbond commented 4 years ago

As with #230 and #231 this is only a first pass; QAQC is long and repetitive, and imho its pieces should be extracted into standalone functions for improved clarity and testing. This PR takes a first few steps in that direction:

jb388 commented 4 years ago

@bpbond re: 2f19387 Question about "warning()": These are written as cat() messages in order to dump the output to the QAQC outfile. If we make these changes can we make sure that still occurs? Generally speaking, the QAQC function has multiple roles:

1) Actually ingesting data and running quality control 2) Enabling R package users to run QA/QC on their own entry files 3) Allowing non-R users to run QA/QC on their own entry files via the web interface

The output file is critical for roles 1 & 3.

bpbond commented 4 years ago

Thanks @jb388 . I wondered about this. I see two options:

  1. Use sink() to divert all output, including warnings, to the output file.
  2. Revert to using cat() throughout.

Number 2 is a bit simpler and maybe, if these warnings are to be treated a bit differently than a standard R warning, is the way to go? 🤷‍♂

jb388 commented 4 years ago

So we had run into issues using sink() in earlier versions of ISRaD. I don't remember the exact issue, but it was something to do with the connection not closing. Our solution was to switch to using an outfile with cat(). Given that experience I'd lean towards keeping the cat() calls.

bpbond commented 4 years ago

Yeah sink() is a big ol' drag to work with, agreed.