metrumresearchgroup / bbi

Next generation modeling platform
11 stars 2 forks source link

Wrong data path when ctl file includes commented out $DATA? #300

Closed kylebaron closed 1 year ago

kylebaron commented 1 year ago

For example,

$DATA ../data/derived/good.csv
; $DATA ../data/derived/bad.csv

I think bbi will write ../data/derived/bad.csv into the bbi_config.json file.

kyleam commented 1 year ago

Thanks for reporting.

The minimal fix could be something like this:

diff --git a/cmd/local.go b/cmd/local.go
index 8135308..1b28ee2 100644
--- a/cmd/local.go
+++ b/cmd/local.go
@@ -10,6 +10,7 @@ import (
    "os/exec"
    "path"
    "path/filepath"
+   "regexp"
    "strings"
    "time"

@@ -213,8 +214,9 @@ func (l LocalModel) Cleanup(channels *turnstile.ChannelMap) {

    log.Debugf("%s Beginning hash calculation operations for data file", l.Nonmem.LogIdentifier())

+   dataRe := regexp.MustCompile(`^\s*\$DATA\s`)
    for _, line := range sourceLines {
-       if strings.Contains(line, "$DATA") {
+       if dataRe.MatchString(line) {
            // extract out data path
            l.Nonmem.DataPath = filepath.Clean(strings.Fields(line)[1])
        }

It would take more work to support the quoted values mentioned in NONMEM's docs:

 filename
      Name  of  the  file  containing  the data set.  Must be the first
      option.  If it contains commas, semicolons, or parentheses,  then
      it  must  be  surrounded  by  single quotes ' or double quotes ".
      Filename may also contain  equal  signs  if  it  is  enclosed  in
      quotes.   If  the file is opened by NM-TRAN, filename may contain
      embedded spaces if it is enclosed in quotes, and may  contain  at
      most  80  characters.  If the file is opened by NONMEM, the file-
      name may not contain embedded spaces, and may contain at most  71
      characters.   If  filename is the same as any option of the $DATA
      record, it must be enclosed in quotes.

This commented out data line will also confuse summarys RunDetails parser:

https://github.com/metrumresearchgroup/bbi/blob/fb3a008198bd14e5b35efbff8eb9c80d2bf53a7d/parsers/nmparser/parse_run_details.go#L87-L88

(That function does a lot of checks like that, so it will have more widespread problems with comments.)