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

correction to commit "Updates to fix soft_masking discripancy - ENSC… #403

Closed
wants to merge 1 commit into from
Closed

Conversation

duartemolha
Copy link

…ORESW"

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion;
  • Review the contributing guidelines for this repository; remember in particular:
    • do not modify code without testing for regression
    • provide simple unit tests to test the changes
    • if you change the schema you must patch the test databases as well, see Updating the schema
    • the PR must not fail unit testing

Description

The commit here:
74e499a

was trying to correct some discrepancies in the softmasking on non-coding sequences
However, completely non-coding exons have a undefined $ex->coding_region_start resulting in warning messages

Use of uninitialized value in numeric gt (>) at ...EnsEMBL/Transcript.pm

After setting all the sequence to lower case at line 837 $exon_seq = lc($exon_seq)
the if statements
if ($ex->coding_region_start($self) > $ex->start()) {
and
if ($ex->coding_region_end($self) < $ex->end()) {
should never be done since both coding_region_start and/or coding_region_end will be undefined if
if (!defined ($ex->coding_region_start($self))) is true

Use case

This bug will output warning messages for completely UTR exons where the optional softmask has been set

for example for gene DDR2 , transcript id ENST00000367921

even though the output softmasking in correct before and after my code change, in the updated code we do not get warning messages such as
Use of uninitialized value in numeric gt (>) at .../ensembl/modules/Bio/EnsEMBL/Transcript.pm line 837.

Benefits

The change I made makes it so that when checking a complete UTR exon (when soft_masking is requested) is all lowercase, and then the comparisons with coding start and coding end with the start and end of the exon are ignored,

Those comparisons are only done IF $ex->coding_region_start is defined
if (!defined ($ex->coding_region_start($self))) {
$exon_seq = lc($exon_seq);
}else{
if ($ex->coding_region_start($self) > $ex->start()) {
...
}

      if ($ex->coding_region_end($self) < $ex->end()) {
        ...
     }

}
$seq_string .= $exon_seq;

Possible Drawbacks

none that I can see

Testing

No. I have not created tests for this. The current tests available test to see if the boundaries between lowerCase and upperCase match.
For exons that are completely UTR, both the previous code and the new code would make the entire exon sequence lowercase. The only difference is that after my change there is no invalid if comparisons with undefined values and therefore no warning messages.
In order to test for this we would need
The code builds fine on the current test suit

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.444% when pulling eedb0e7 on duartemolha:release/97 into 378db18 on Ensembl:release/97.

@duartemolha duartemolha closed this Aug 2, 2019
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.

2 participants