Conversation
components/config/src/nonmem.rs
Outdated
| suffix.split('_').collect() | ||
| }; | ||
|
|
||
| Some((version_number, suffix_parts)) |
There was a problem hiding this comment.
can you add some docstring to explain what the suffixes are? I can see it from the tests but it would be nice to be in the docs
components/config/src/nonmem.rs
Outdated
| match (parse_nonmem_version_name(a), parse_nonmem_version_name(b)) { | ||
| (Some((a_num, a_suffix)), Some((b_num, b_suffix))) => b_num | ||
| .cmp(&a_num) | ||
| .then_with(|| a_suffix.cmp(&b_suffix)) |
There was a problem hiding this comment.
Does it make sense to compare anything other than version number? If you have nm73 and nm73gf they are not the same? what does the ordering mean there?
There was a problem hiding this comment.
Yeah I think you're right we can just ignore the suffix entirely. parse the version number then lexical for ties. The suffixes just denoted how the fortran was compiled.
| job_id_str | ||
| .parse() | ||
| .map_err(|e| anyhow!("Failed to parse job ID '{job_id_str}': {e}"))? | ||
| } |
There was a problem hiding this comment.
this block needs some comment
| let b_num = parse_nonmem_version_name(b); | ||
|
|
||
| match (a_num, b_num) { | ||
| (Some(a_num), Some(b_num)) => b_num.cmp(&a_num).then_with(|| a.cmp(b)), |
There was a problem hiding this comment.
without it, it could put versions with the compiler information before the plain nm7x.
assertion `left == right` failed
left: ["nm76", "nm75", "nm74gf_nmfe", "nm74", "nm74gf", "nm73", "nm73gf", "nm73gf_nmfe"]
right: ["nm76", "nm75", "nm74", "nm74gf", "nm74gf_nmfe", "nm73", "nm73gf", "nm73gf_nmfe"]
I'm not sure if this is a concern in practice, but feels like it nm7x should take precedence over nm7x_y
2 participants
No description provided.