lucapinello / CRISPResso

Software pipeline for the analysis of CRISPR-Cas9 genome editing outcomes from sequencing data
Other
131 stars 55 forks source link

If guide seq is wrong, do not crash instead run without guide seq #21

Closed Imoteph closed 7 years ago

Imoteph commented 7 years ago

Thanks for the software and the recent addition of allowing outies for flash ! TOP :+1:

Would you maybe as well considering following change as well in the Core code?

                     if not cut_points:
                         #CHANGE ADDED here (add warning and default values instead of a crash)
                         #raise SgRNASequenceException('The guide sequence/s provided is(are) not present in the amplicon sequence! \n\nPlease check your input!')
                         warn('The guide sequence/s provided is(are) not present in the amplicon sequence! \n\nPlease check your input!, running now without guide_sequence!!')
                         cut_points=[]
                         sgRNA_intervals=[]
                         offset_plots=[]
                     else:
                         info('Cut Points from guide seq:%s' % cut_points)

This helps for throughput analysis. I know that the downstream calculation values are off, but the resulting allele_frequency_tsv should not be affected at all. What do you think?

lucapinello commented 7 years ago

Hi Imoteph,

yeah I thought about this option. In a previous version of CRISPResso I had this logic impemented but this created a lot of confusion, since people don't usually read the full log (especially if you are using the CRISPRessoPooled with multiple guides). I think it is better to be explicit in this case, since if you design the experiment you should know the sgRNA sequence and if you are providing the wrong one it is better to stop the execution to be on the safe side.

The allele_frequency_tsv will show the same rows but the classification of modified or unmodified may be different.

Imoteph commented 7 years ago

Hi lucapinello,

Thanks for your answer! I already were thinking you would say this. But in our case we have to deal with some flexibility regarding the guide_seq which could contain one or two (not around the PAM site) SNPs. Therefore I changed this into a warning instead of crashing. But I am thinking already to modify it towards a more fuzzy search (allowing one or two SNPs) of the guide_seq, which would allow as well the correct classification. I will let you know, when I find time to do this, maybe this can be included as well as additional option.

lucapinello commented 7 years ago

Oh I see, that is a good point I didn't think about this.

Also be careful even with the warning the quantification may be off since you have two reference alleles and not one. I will think about this use case.

Thanks!

On Fri, Jul 28, 2017 at 9:29 AM, Vipul Kumar Patel <notifications@github.com

wrote:

Hi lucapinello,

Thanks for your answer! I already were thinking you would say this. But in our case we have to deal with some flexibility regarding the guide_seq which could contain one or two (not around the PAM site) SNPs. Therefore I changed this into a warning instead of crashing. But I am thinking already to modify it towards a more fuzzy search (allowing one or two SNPs) of the guide_seq, which would allow as well the correct classification. I will let you know, when I find time to do this, maybe this can be included as well as additional option.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/lucapinello/CRISPResso/issues/21#issuecomment-318651973, or mute the thread https://github.com/notifications/unsubscribe-auth/ABB_6sbuahPNWWIYgVtJX1dLgWbRKOn0ks5sSeI5gaJpZM4OfWcT .