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

feat: flag MHC credible sets based on lead #767

Merged
merged 11 commits into from
Sep 18, 2024
Merged

feat: flag MHC credible sets based on lead #767

merged 11 commits into from
Sep 18, 2024

Conversation

d0choa
Copy link
Collaborator

@d0choa d0choa commented Sep 17, 2024

This PR closes opentargets/issues#3469.

It implements:

  • QC flag for MHC region in StudyLocus
  • Add flag in study_locus_validation step
  • New GenomicRegion class refactoring some pre-existing code

It's pending:

  • Add new flag IN_MHC to orchestration config

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Sep 17, 2024
@d0choa d0choa marked this pull request as ready for review September 17, 2024 19:28
@d0choa
Copy link
Collaborator Author

d0choa commented Sep 17, 2024

@DSuveges and @Daniel-Considine, this touches on code that you worked on in the past, so please feel free to raise any objections. I'm assigning the review to Szymon and Yakov. (I didn't want to assign everyone 😅)

@Daniel-Considine
Copy link
Contributor

Looks all good to me

Copy link
Contributor

@DSuveges DSuveges left a comment

Choose a reason for hiding this comment

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

There's a comment in the src/gentropy/study_locus_validation.py file indicating a filtering for MHC, however I cannot find the logic to do so. Is it intentional? I suspect the comment should go, and a exclusion flag needs to be added to the orchestration.

@d0choa
Copy link
Collaborator Author

d0choa commented Sep 18, 2024

  • Comment updated
  • As said at the top, we need to add Add new flag IN_MHC to orchestration config, and that would be enough to handle the filtering using the generic valid_rows function.

Copy link
Contributor

@DSuveges DSuveges left a comment

Choose a reason for hiding this comment

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

Now the flagging comment is duplicated, but this is no way block the PR.

@d0choa
Copy link
Collaborator Author

d0choa commented Sep 18, 2024

that's why I shouldn't do commits from the web. Fixed now

@d0choa d0choa merged commit 8c4421a into dev Sep 18, 2024
4 checks passed
@d0choa d0choa deleted the do_mhc_qc branch September 18, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dataset documentation Improvements or additions to documentation Feature size-M Step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flag and then filter MHC region from credible sets
4 participants