normalize-by-median.py --force default should be 'true'

[macmanes]

In messing around with normalize-by-median.py, I realized that if I killed a run while it was running, then issued a new command using the same output filename, that the product of the new command was appended to the results of the first command. I don't think this is ever when the user wants - a set of reads will be duplicated in the normalized output file.. I think that the output should always be overwritten --force TRUE with or without a warning that the file is being overwritten..

grep HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129 khmer.normalized.C50.fastq
>HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129/1
>HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129/2
>HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129/1
>HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129/2

My 2cents

[ctb]

On Wed, Jan 28, 2015 at 12:11:08PM -0800, Matt MacManes wrote:

In messing around with normalize-by-median.py, I realized that if I killed a run while it was running, then issued a new command using the same output filename, that the product of the new command was appended to the results of the first command. I don't think this is ever when the user wants - a set of reads will be duplicated in the normalized output file.. I think that the output should always be overwritten --force TRUE with or without a warning that the file is being overwritten..

grep HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129 khmer.normalized.C50.fastq
>HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129/1
>HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129/2
>HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129/1
>HWI-ST1097:199:D22V1ACXX:1:1101:1154:2129/2

My 2cents

Definitely a bug. thx!

--titus

[ctb]

I think the fix is to move this open out of the for loop. It's a little sketchy it's in the for loop, frankly :)

https://github.com/ged-lab/khmer/blob/master/scripts/normalize-by-median.py#L243

[drtamermansour]

I will be working on this #745 .
I agree we have to move the open of the output file to outside the loop. However, this is not going to solve the problem. The basic idea here is that, as we go through the input files, we append the reads to a single output file.
The default --force as false allow the user to keep adding normalized reads to the same output file from more input files. I think we should change the default --force to True.
Also we can add a check when we write the 1st read to the output file and through a warning if the read already exists in the output

[ctb]

I'm -1 on having 'force' be a default - what's the point of it, then? :) I would suggest moving the default to do what's right, but you'll have to confer with @mr-c about whether or not that breaks semantic versioning.

[mr-c]

I would call this a bug and that the fix would not break semantic versioning. I will prominently advertise the change in behaviour upon release.

[drtamermansour]

This function does not make sense for me:
https://github.com/ged-lab/khmer/blob/master/scripts/normalize-by-median.py#L221
I tracked the implementation of the function here
https://github.com/drtamermansour/khmer/blob/fix/%23745/khmer/kfile.py#L43
It uses the size of the input files to make a decision about the available desk space !!!

[ctb]

Yes, that all seems correct to me - what do you disagree with?