clean up print_clonotypes #409
Conversation
| @@ -359,7 +345,7 @@ pub fn print_clonotypes( | |||
| // Generate Loupe data. | |||
|
|
|||
| if (!ctl.gen_opt.binary.is_empty() || !ctl.gen_opt.proto.is_empty()) && pass == 2 { | |||
| loupe_clonotypes.push(make_loupe_clonotype( | |||
There was a problem hiding this comment.
Ah, this was always a vector with 0 or 1 elements, so we changed it to an option. Makes sense
There was a problem hiding this comment.
Yeah, there are a lot of places in this codebase where a vector is used as a proxy for optionality. It is confusing at first but ends up making way more sense once refactored to an option, especially in cases where several vectors represent several items that are all either present or absent together. I have more clean-up along these lines in subsequent patches.
There was a problem hiding this comment.
This section of code is kind of confusing because it has this two-pass loop with a lot of pass-wise conditionals. I'm unrolling this in follow-up code to make it easier to understand what's happening.
| for ri in &results { | ||
| for vj in &ri.11 { | ||
| fate[vj.0].insert(vj.1.clone(), vj.2.clone()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Oops, good catch! I tried to fold all of the multiple for-loops into a single pass and it looks like I missed this one. I will restore this code.
There was a problem hiding this comment.
Oh, no, I know why this went away - the 11th field of that results tuple was initialized with an empty vector but it was never actually written to anywhere.
There was a problem hiding this comment.
Put another way - this section of code doesn't actually modify fate. The mutability was only necessary because of this little piece of code which was actually dead, just not in a way that the compiler could notice. I remove the mutability in a subsequent PR.
Tidies up the implementation of print_clonotypes in preparation for additional refactoring.