yatisht / usher

Ultrafast Sample Placement on Existing Trees
MIT License
121 stars 41 forks source link

non-intuitive behavior for empty subsets, arguably a bug #268

Closed jbloom closed 1 year ago

jbloom commented 1 year ago

Running the command:

matUtils extract -i {input.mat} -s {input.samples} -o {output.mat}

where {input.samples} is an empty file returns the entire mutation-annotated tree.

I feel like this is a non-intuitive behavior, as the -s command is supposed to be for subsetting for the specified samples. I think it would be more intuitive to either return an empty mutation-annotated tree of raise an error.

(This is obviously most relevant to pipelines automatically generating different subsets, where if one happens to be empty instead you get back the whole mutation annotated tree.)

yatisht commented 1 year ago

@jmcbroome this should be an easy fix I think. Could you please have a look?

jmcbroome commented 1 year ago

Just added handling for this to a PR. Sorry I didn't consider that edge case before- thank you for raising the issue!

yatisht commented 1 year ago

PR merged!

jbloom commented 1 year ago

Great, thanks for fixing this!