Fix empty output files when no targets are found in inputs #51
Conversation
Signed-off-by: pkruczkiewicz <peter.kruczkiewicz@canada.ca>
Signed-off-by: pkruczkiewicz <peter.kruczkiewicz@canada.ca>
peterk87
changed the title
| scheme_subtype_counts=scheme_subtype_counts, | ||
| n_threads=n_threads) | ||
| logging.info('Generated %s subtyping results from %s contigs samples', len(contigs_results), len(input_contigs)) | ||
| subtype_results += contigs_results |
There was a problem hiding this comment.
Is subtype_results iterable? did you mean subtype_results.append(contigs_results)?
There was a problem hiding this comment.
It's a nice thing you can do in Python where you can add 2 lists together with +:
list1 = [1, 2, 3]
list1 += [4, 5]
assert list1 == [1,2,3,4,5]
#or
list1 = [1, 2, 3]
list2 = [4, 5]
list3 = list1 + list2
assert list3 == [1,2,3,4,5]
# list1 and list2 are unmodifiedHopefully that makes sense!
There was a problem hiding this comment.
oh ok I see, that makes sense, thanks!
| scheme_subtype_counts=scheme_subtype_counts, | ||
| n_threads=n_threads) | ||
| logging.info('Generated %s subtyping results from %s contigs samples', len(reads_results), len(input_reads)) | ||
| subtype_results += reads_results |
bio_hansel/main.py
Outdated
| from typing import Optional, List, Any, Tuple | ||
|
|
||
| import attr | ||
| import os |
There was a problem hiding this comment.
Isn't "os" a built-in module, shouldn't it be in the first group?
There was a problem hiding this comment.
Yup! Fixed in acf4335
| input_genomes, reads = collect_inputs(args) | ||
| if len(input_genomes) == 0 and len(reads) == 0: | ||
| input_contigs, input_reads = collect_inputs(args) | ||
| if len(input_contigs) == 0 and len(input_reads) == 0: |
There was a problem hiding this comment.
Should this be an "or" statement, that if either of these are empty, then it raises the error? Or is it ok if one of them is empty, that's still ok?
There was a problem hiding this comment.
We're allowing the user to specify a mix of file types. They can specify contigs (FASTA) and reads (FASTQ) in the same analysis. If both lists are empty then no input whatsoever has been specified so we can't do anything except let the user know they haven't specified anything to analyze.
organize imports
bio_hansel/qc/checks.py
Outdated
| @@ -4,10 +4,10 @@ | |||
|
|
|||
| from pandas import DataFrame | |||
There was a problem hiding this comment.
For consistency, would it be good to import pandas as pd just like the other files?
There was a problem hiding this comment.
Yes, you're right. Why from pandas import DataFrame where everywhere else it's import pandas as pd? There's no good reason for that to remain the way it is. Fixed in 0a8fe89
Signed-off-by: pkruczkiewicz <peter.kruczkiewicz@canada.ca>
Signed-off-by: pkruczkiewicz <peter.kruczkiewicz@canada.ca>
|
Great! @gcttong If it looks good to you, then can you merge the changes? Thanks for taking a look at this PR. |
This pull request: