makovalab-psu/NoiseCancellingRepeatFinder

Potential bug

seryrzu opened this issue · 4 comments

Line 379 ncrf_consensus_filter.py:
motif = reverse_complement(motifLen)
should that be
motif = reverse_complement(motif)?

and probably line 380 should be for rotIx in xrange(motifLen)

I'm running a command like that:

 ~/soft/NoiseCancellingRepeatFinder/ncrf_cat.py NCRF/report.txt | ~/soft/NoiseCancellingRepeatFinder/ncrf_consensus_filter.py --consensusonly

and get

Traceback (most recent call last):
  File "/home/abzikadze/soft/NoiseCancellingRepeatFinder/ncrf_consensus_filter.py", line 477, in <module>
    if __name__ == "__main__": main()
  File "/home/abzikadze/soft/NoiseCancellingRepeatFinder/ncrf_consensus_filter.py", line 96, in main
    simple_consensus_filter(stdin)
  File "/home/abzikadze/soft/NoiseCancellingRepeatFinder/ncrf_consensus_filter.py", line 129, in simple_consensus_filter
    seqChunks = chunkify(a.motif,motifText,seqText)
  File "/home/abzikadze/soft/NoiseCancellingRepeatFinder/ncrf_consensus_filter.py", line 315, in chunkify
    (motifPos,direction) = position_in_motif(motif,motifText)
  File "/home/abzikadze/soft/NoiseCancellingRepeatFinder/ncrf_consensus_filter.py", line 379, in position_in_motif
    motif = reverse_complement(motifLen)
  File "/home/abzikadze/soft/NoiseCancellingRepeatFinder/ncrf_parse.py", line 320, in reverse_complement
    return nukes[::-1].translate(complementMap)
TypeError: 'int' object has no attribute '__getitem__'

Rats! Thanks for finding that.

I've updated the repo with a correction.

Obviously I did a poor job of testing that module. What has me scratching my head is why I never encountered that issue in my own use. That module was a late addition to the package, and apparently in my tests the motif must have always been found, in this context, in forward orientation.

After some additional testing, I intend to tag this as a new release, and update the bioconda recipe.

Thanks again for reporting that.

Not clear that this fix is entirely correct though. I am still testing.

I see why I didn't encounter it. If the input alignment file indicates a strand for the motif (which IIRC is optional), simple_consensus_filter() flips the alignment (at line 124). So by the time it gets to position_in_motif() the alignment is in the same orientation as the motif, and the call is resolved before it gets to the buggy part.

I'm still confused as to why I didn't catch that bug. Back on line 121 I see that I have an unadvertised mechanism to prevent that strand flip. And if I use that (now) without the fix, I get the exception you reported. I wouldn't have added that mechanism without using it.

@seryrzu I've committed a second correction for this.

While the first commit prevented the exception, downstream processing had another bug. That would result in no consensus being called (for alignments that would have previously triggered that exception).

I've corrected that.

Thanks for finding that.