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

Fix for #519 - add support for HGVS repeats #528

Merged
merged 2 commits into from
Jun 24, 2021

Conversation

markwoon
Copy link
Contributor

Took a stab at implementing support for repeats as documented at https://varnomen.hgvs.org/recommendations/DNA/variant/repeated/.

I hope the new class names sufficiently aligns with what you expect.

I considered renaming the NucleotideShortSequenceRepeatVariability over to LegacyNucleotideShortSequenceRepeatVariability but figured this would be a breaking change. I'm not sure how you want to distinguish between the current spec and the old spec at https://www.hgvs.org/mutnomen/.

@holtgrewe holtgrewe self-assigned this Jun 18, 2021
@holtgrewe
Copy link
Member

Dear @markwoon, thanks for this 👏. I glanced over this quickly and I have to say: good job, in particular getting into the Antlr part is not trivial IMO. I will need to take some time to review this (next week).

Cheers!

@markwoon
Copy link
Contributor Author

Learning antlr was fun.

I am torn between writing my own parser and using jannovar. I'd like to just use jannovar but I'm starting to need the parts of the spec that jannovar doesn't cover.

I'm happy to contribute, but I'm not sure what your plan is for fully supporting the current spec, and how/if you plan on supporting the older spec. I would be interested in working on a version that only has to care about the current spec.

Copy link
Member

@holtgrewe holtgrewe left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@holtgrewe
Copy link
Member

Hi Mark,

thanks again, i just merged it.

Maybe we continue the discussion on HGVS support in #519?

Cheers!

@holtgrewe holtgrewe merged commit ed63c12 into charite:develop Jun 24, 2021
@holtgrewe
Copy link
Member

Oh, by the way, do you need a release from this or are you building yourself anyway?

@markwoon
Copy link
Contributor Author

Would prefer an official release, but I'm using my own build in the meantime.

@holtgrewe
Copy link
Member

holtgrewe commented Jun 24, 2021

@markwoon Then let's see where #519 takes us. I'm a bit reluctant to release for such small features (Jannovar build infrastructure is not properly setup for such small-stepped builds) but would do a release if we don't expect any changes of the code base in the next weeks.

edit: s/days/weeks/

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