metrumresearchgroup / bbi

Next generation modeling platform
11 stars 2 forks source link

init: warn on ID collisions and support NONMEM directories with periods #304

Closed kyleam closed 1 year ago

kyleam commented 1 year ago

This series was prompted by bbi init mishandling a directory with periods. For example, a directory with the name "7.5.0" will lead to this entry:

nonmem:
  "7": {}
demo script ```sh #!/bin/sh set -eu tdir=$(mktemp -d "${TMPDIR:-/tmp}"/bbi-init-test-XXXXXXX) test $# != 0 || { echo "usage: $0 " exit 1 } cd "$tdir" for nm_name in "$@" do for sdir in license run util source do mkdir -p d/"$nm_name"/"$sdir" done :> d/"$nm_name"/license/nonmem.lic :> d/"$nm_name"/run/nmfe75 chmod +x d/"$nm_name"/run/nmfe75 done bbi init --dir d cat bbi.yaml ``` ``` ./demo.sh 7.5.0 | grep nonmem -A1 nonmem: "7": {} ```

After the last commit, that will be accessible as "7-5-0".


init: rename variable for style consistency
init_test: check that error is non-nil before inspecting message
init: convert a panic to a returned error

These first three commits touch up minor things in the area that aren't directly related to the original problem.

init: extract logic for finding installation directories to function
init: generate NonMemDetail map directly

The next commits split up and reworks the implementation a bit, mostly for easier testing (but I also think it makes things a bit clearer).

init: warn on identifier collisions
init: detect collision due case insensitive keys

These two commits add collision warnings for two cases that could be hit before this PR.

init: sanitize config identifier for nonmem entries

The last commit handles the "." to "-" replacement, warning if that creates any collisions.