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

[GEN-1067] add functionality to warn for identical ref and tsa2 #553

Merged
merged 8 commits into from
Mar 14, 2024

Conversation

danlu1
Copy link
Contributor

@danlu1 danlu1 commented Mar 9, 2024

Purpose:
This PR includes changes in _check_tsa1_tsa2 function of maf.py to throw errors (column level and row level) when REFERENCE_ALLELE is equal to TUMOR_SEQ_ALLELE2.

Test:

  1. Unit tests are included.
  2. Updated GOLD center file and Test center file and ran validation locally and on ec2 instance.

@danlu1 danlu1 requested a review from a team as a code owner March 9, 2024 08:17
remove extra trailing whitespaces
Copy link
Contributor

@rxu17 rxu17 left a comment

Choose a reason for hiding this comment

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

Great first genie PR! I added some comments

genie_registry/maf.py Outdated Show resolved Hide resolved
tests/test_maf.py Outdated Show resolved Hide resolved
@rxu17
Copy link
Contributor

rxu17 commented Mar 11, 2024

Also don't forget to make sure your build is successful, i think there's just some formatting to adjust in your code: https://github.com/Sage-Bionetworks/Genie/actions/runs/8237606927/job/22526728071?pr=553

If you have black installed, you can just run black on everything using black . (which will auto-lint your code) then commit and push the formatting changes.

genie_registry/maf.py Outdated Show resolved Hide resolved
@rxu17 rxu17 self-requested a review March 13, 2024 19:25
Copy link
Contributor

@rxu17 rxu17 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for implementing the changes. Going to pre-approve! Just a couple more comments.

Copy link

Quality Gate Passed Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@danlu1 danlu1 merged commit d190e61 into develop Mar 14, 2024
8 checks passed
@rxu17 rxu17 deleted the GEN_1067_warn_for_unchanged_mutations branch March 20, 2024 18:44
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