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

Variant Recoder - remove duplicated MANE transcripts #1018

Merged
merged 5 commits into from Sep 14, 2021

Conversation

dglemos
Copy link
Contributor

@dglemos dglemos commented Jul 7, 2021

mane_select returns duplicated results. Example: LAMA3:c.4998+1439G>A

@coveralls
Copy link

coveralls commented Jul 7, 2021

Coverage Status

Coverage increased (+0.01%) to 98.511% when pulling 3df88ad on dglemos:vr_fix_mane_duplicates into 8e7c835 on Ensembl:postreleasefix/105.

@dglemos dglemos closed this Jul 7, 2021
@dglemos dglemos reopened this Jul 7, 2021
@dglemos dglemos closed this Jul 7, 2021
@dglemos dglemos reopened this Jul 7, 2021
@dglemos dglemos added on hold and removed on hold labels Jul 9, 2021
@helensch helensch added the e105 label Sep 10, 2021
@helensch helensch self-assigned this Sep 10, 2021
Copy link
Contributor

@helensch helensch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code removes the duplicated MANE transcripts.
There are only a couple of small queries.
As a new test has been added, the number of test skipped should be updated in VariantRecoder.t

@@ -374,6 +374,21 @@ SKIP: {
'recode - output MANE Select and fields'
);

my $vr_mane_hgvs = Bio::EnsEMBL::VEP::VariantRecoder->new({%$cfg_hash, %$db_cfg, offline => 0, database => 1, species => 'homo_vepiens', mane_select => 1, fields => 'spdi'});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new test reports
'Use of uninitialized value in subtraction (-) at ensembl/modules/Bio/EnsEMBL/TranscriptMapper.pm line 384 and line 384. This is for $self->{'cdna_coding_start'}. Is this expected?

Also get warnings when run variant_recoder with "GABPA:p.Trp189Ter" before and after the PR change.

Copy link
Contributor Author

@dglemos dglemos Sep 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About the first warning in TranscriptMapper.pm, I'm going to open a ticket to investigate further. We found issues with TranscriptMapper before it could be related. Second warning: I'll add the example to the same ticket.

$mane_object{'hgvsc'} = $mane_hgvsc;
$mane_object{'hgvsp'} = $mane_hgvsp;
push @{$mane_by_allele{$key_allele}->{'mane_select'}}, \%mane_object;
my $key_hgvsg = $object->{'hgvsg'};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initial code had my $mane_hgvsg = $object->{'hgvsg'} || '-';
in my $key_hgvsg = $object->{'hgvsg'}; is $object->{'hgvsg'} || '-'; needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$object->{'hgvsg'} is supposed to be always defined that is why I removed my $mane_hgvsg = $object->{'hgvsg'} || '-';

@helensch helensch merged commit 16e800f into Ensembl:postreleasefix/105 Sep 14, 2021
@helensch
Copy link
Contributor

merged into master and release/105
For master had to resolve merge conflicts due to number of tests and ga4gh_vrs tests in t/VariantRecoder.t

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants