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

#139 updated codeQL version to v2 and #3482 Fix user searches for var… #854

Merged
merged 3 commits into from
Jul 31, 2023

Conversation

Bharath-kandula
Copy link
Contributor

…iants where the N is accidentally ommitted from NM_

…iants where the N is accidentally ommitted from NM_
@TheMadBug
Copy link
Member

@Bharath-kandula It works well when searcing for uppercase, but doesn't work for "m_000546.5(TP53):c.1129A>C" for example.
If you make the M_ check case insensitive and add a unit test for that, should be good to go.

Also good to see that the basics of CodeQL is working again.

…riants where the N is accidentally ommitted from NM_
@@ -516,6 +516,8 @@ def clean_hgvs(self, hgvs_string) -> Tuple[str, List[str]]:
cleaned_hgvs = clean_string(hgvs_string) # remove non-printable characters
cleaned_hgvs = cleaned_hgvs.replace(" ", "") # No whitespace in HGVS
cleaned_hgvs = cleaned_hgvs.replace("::", ":") # Fix double colon
if cleaned_hgvs.startswith("M_") or cleaned_hgvs.startswith("m_"):
Copy link
Member

Choose a reason for hiding this comment

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

Really minor coding style preference:
Could you change this to
if cleaned_hgvs[0:2].upper() == "M_"
to avoid making a check for M_ and m_. Just saves us repeating ourselves a little bit.

…riants where the N is accidentally ommitted from NM_
@TheMadBug TheMadBug merged commit bba5cf3 into master Jul 31, 2023
3 checks passed
@TheMadBug TheMadBug deleted the #139_update_codeQL_version branch August 31, 2023 23:43
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