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

Remove POSIX ceil dep from Downstream #371

Merged
merged 6 commits into from
Sep 6, 2021
Merged

Conversation

stekaz
Copy link
Contributor

@stekaz stekaz commented Dec 8, 2020

I'm not sure if this is really all that helpful, I hope that it is so I thought I'd submit a PR anyway. I refactored the Downstream plugin a few months ago but got busy with other projects. It removes the dependency on POSIX ceil and simplifies how the sequence "to translate" is obtained. In my limited testing, it produced the same results as the current version (v2.3). I found this PR helpful when I was debugging pVACtools (please see #342). I wonder if this could be helpful for you @at7?

@aparton aparton requested a review from at7 January 18, 2021 15:33
@aparton
Copy link
Contributor

aparton commented Jan 18, 2021

Hi @stekaz,

Thank you for submitting this PR, we always welcome plugin submissions from users. We'll review it in the coming weeks and let you know if we have any questions.

Thanks,
Andrew

@at7 at7 self-assigned this Jan 25, 2021
@at7 at7 requested a review from aparton February 4, 2021 11:26
Copy link
Contributor

@at7 at7 left a comment

Choose a reason for hiding this comment

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

The PR presents a cleaner organisation of the code and simplifies updating CDS sequence with TVA sequence and retrieving CDS sequence after last codon which doesn't overlap the variant allele. It also removes the POSIX dependency. PR adds check that FASTA file is provided in offline mode.
Major code restructure relates to:
-- replaces CDS sequence with TVA sequence, handling insertions correctly
-- computes last_complete_codon end position using modulo, replacing the dependency on ceil function which also works if CDS start overlaps the first codon
-- gets the downstream sequence starting after the last complete codon

-- following steps haven't changed
-- Updated plugin returns same results as it did before the updates. Plugin versions were tested with all frameshift variants for COL7A1 (on reverse strand) and BRCA2 (forward strand)

@at7 at7 merged commit a2def0d into Ensembl:master Sep 6, 2021
@stekaz stekaz deleted the simple_downstream branch September 6, 2021 23:34
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