Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in is_ter_codon #387

Closed
wants to merge 2 commits into from
Closed

Bug in is_ter_codon #387

wants to merge 2 commits into from

Conversation

Juke34
Copy link
Contributor

@Juke34 Juke34 commented Feb 16, 2024

is_ter_codon and is_start_codon do not behaves the same. ( with that PR now it does).
is_ter_codon was not verifying all unambiguous_codons, but only the first.

I do not get why Matthew Laird that coded is_ter_codon in 2016 did it like that while the _codon_is was existing and used by is_start_codon since 2010.

sub is_start_codon{
   shift->_codon_is( shift, \@STARTS, 'M' );
}

How is_ter_codon was coded was not behaving as explained. Indeed only the first codon for ambiguous codon was tried. It was coded Upside down (between the else and if statement).

       my $result = 0;

       # For all the possible codons, if any are not a stop
       # codon, fail immediately
       for my $c ( $self->unambiguous_codons($value) ) {
	   my $m = substr( $TABLES[$id], $CODONS->{$c}, 1 );
	   if($m eq $TERMINATOR) {
	       $result = 1;
	   } else {
	       return 0;
	   }
       }
       return $result;

Anyway using the _codon_is method as for is_start_codon seems perfectly fine.

…is_ter_codon was not verifying all unambiguous_codons, but only the first.
@cjfields
Copy link
Member

cjfields commented May 6, 2024

Hi @Juke34 there appear to be a few conflicts with the master branch. Are these from your local checkout of the code?

@carandraug
Copy link
Member

I think I already fixed this issue last week with the following series:

  1. 7a28711
  2. 17f4c3b
  3. fa9366f

(but maybe your fix covers more stuff? Please take a look)

@carandraug
Copy link
Member

I see now that this was from 3 months ago. Oh well, when I fixed I was aiming at closing #266 which reports the same issue. Closing as fixed.

@carandraug carandraug closed this May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants