Improve the cigar handling of ClipAdapters.

[gt1]

Andrew, it looks like you are adding to the hard clipping operation for operations which are not hardclip. Is this intentional?

if ( cig_type == 0 )
{
            hardclip += cigop[index].second;
}

cig_type can be 0 for cigar operations which are not hard clip.

[gt1]

two more things:

1. it would be nice to use symbols instead of numbers where possible (e.g. libmaus2::bambam::BamFlagBase::LIBMAUS2_BAMBAM_CHARD_CLIP instead of 5). This would make the code more readable.
2. if this includes actual code from htslib, then this would need to be mentioned in a license header or at least the AUTHORS file

[whitwham]

Hi German,

Thanks for taking a look so quickly.

On 21/09/15 20:31, German Tischler wrote:

Andrew, it looks like you are adding to the hard clipping operation for operations which are not hardclip. Is this intentional?

if ( cig_type == 0 )
{
              hardclip += cigop[index].second;
}

cig_type can be 0 for cigar operations which are not hard clip.

It is intentional though I am not sure it is the right thing to do. It is a simplification but it does keep the maths right in terms of position and remaining sequence. Information on the previous alignment is lost but it is lost anyway in the hard clip, there is no mechanism for preserving it.

The old code gives a bad cigar string and incorrect alignments, I think this method gives better results.

two more things:

1. it would be nice to use symbols instead of numbers where possible (e.g. libmaus2::bambam::BamFlagBase::LIBMAUS2_BAMBAM_CHARD_CLIP instead of 5). This would make the code more readable.

Agreed. That line of code was already there so I left it alone. I'll make the changes.

1. if this includes actual code from htslib, then this would need to be mentioned in a license header or at least the AUTHORS file

It is actual code from htslib. How do you want to handle this? I can re-write it to use my own methods if you prefer it that way.

regards,

Andrew

The Wellcome Trust Sanger Institute is operated by Genome Research Limited, a charity registered in England with number 1021457 and a company registered in England with number 2742969, whose registered office is 215 Euston Road, London, NW1 2BE.

[gt1]

Hi Andrew,

On 22.09.2015 11:26, Andrew Whitwham wrote:

Hi German,

Thanks for taking a look so quickly.

On 21/09/15 20:31, German Tischler wrote:

Andrew, it looks like you are adding to the hard clipping operation for operations which are not hardclip. Is this intentional?

if ( cig_type == 0 )
{
hardclip += cigop[index].second;
}

cig_type can be 0 for cigar operations which are not hard clip.

It is intentional though I am not sure it is the right thing to do. It is a simplification but it does keep the maths right in terms of position and remaining sequence. Information on the previous alignment is lost but it is lost anyway in the hard clip, there is no mechanism for preserving it.

The old code gives a bad cigar string and incorrect alignments, I think this method gives better results.

ok, if it works for you.

two more things:

1. it would be nice to use symbols instead of numbers where possible (e.g. libmaus2::bambam::BamFlagBase::LIBMAUS2_BAMBAM_CHARD_CLIP instead of 5). This would make the code more readable.

Agreed. That line of code was already there so I left it alone. I'll make the changes.

Thanks.

1. if this includes actual code from htslib, then this would need to be mentioned in a license header or at least the AUTHORS file

It is actual code from htslib. How do you want to handle this? I can re-write it to use my own methods if you prefer it that way. I think there should be a license header in the file. Which license does htslib use?

Best German

[whitwham]

Hi German,

I think there should be a license header in the file. Which license does htslib use?

It uses MIT/Expat License, the following is the terms from the htslib LISCENSE file.

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

With the author in the sam.h file as Heng Li lh3@sanger.ac.uk

regards,

Andrew

The Wellcome Trust Sanger Institute is operated by Genome Research Limited, a charity registered in England with number 1021457 and a company registered in England with number 2742969, whose registered office is 215 Euston Road, London, NW1 2BE.

[gt1]

Hi Andrew,

On 22.09.2015 13:43, Andrew Whitwham wrote:

Hi German,

I think there should be a license header in the file. Which license does htslib use?

It uses MIT/Expat License, the following is the terms from the htslib LISCENSE file.

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

With the author in the sam.h file as Heng Li lh3@sanger.ac.uk

could you please add the license text and appropriate copyright holder statements in the file? I have updated the COPYING and AUTHORS files.

Thanks German

[whitwham]

could you please add the license text and appropriate copyright holder statements in the file? I have updated the COPYING and AUTHORS files.

Done. Also added copyright to ClipAdapter.hpp.

Andrew

The Wellcome Trust Sanger Institute is operated by Genome Research Limited, a charity registered in England with number 1021457 and a company registered in England with number 2742969, whose registered office is 215 Euston Road, London, NW1 2BE.

[gt1]

Thanks.