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: fix biosample study validation #810

Merged
merged 5 commits into from
Oct 3, 2024

Conversation

Tobi1kenobi
Copy link
Contributor

@Tobi1kenobi Tobi1kenobi commented Oct 3, 2024

✨ Context

Fixes a bug in the study validation where checking studies for biosample index would drop all GWAS studies. Also implements additional testing to catch this scenario in the future.

🛠 What does this PR implement

Changed the logic of the biosample_index study validation slightly to match the gene_index study validation a bit closer. Also modified the TestGeneValidation suite to be more abstract and inclusive i.e. be applicable to any study validation that is only for QTLs.

🙈 Missing

One function is duplicated instead of abstracted as the amount of code written was similar so I thought perhaps better to keep simple.

🚦 Before submitting

  • Do these changes cover one single feature (one change at a time)?
  • Did you read the contributor guideline?
  • Did you make sure to update the documentation with your changes?
  • Did you make sure there is no commented out code in this PR?
  • Did you follow conventional commits standards in PR title and commit messages?
  • Did you make sure the branch is up-to-date with the dev branch?
  • Did you write any new necessary tests?
  • Did you make sure the changes pass local tests (make test)?
  • Did you make sure the changes pass pre-commit rules (e.g poetry run pre-commit run --all-files)?

@Tobi1kenobi Tobi1kenobi changed the title Alegbe fix biosample study validation [opentargets/issues/3559] Fix biosample study validation Oct 3, 2024
@Tobi1kenobi Tobi1kenobi changed the title [opentargets/issues/3559] Fix biosample study validation feat: fix biosample study validation Oct 3, 2024
@Tobi1kenobi Tobi1kenobi linked an issue Oct 3, 2024 that may be closed by this pull request
@Tobi1kenobi Tobi1kenobi marked this pull request as ready for review October 3, 2024 09:24
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.

This PR updates the biosample validation logic:

  • A new column (biosampleId) is added to the studyIndex schema.
  • Only non-gwas studies are validated for biosamples.
  • The validation adds a new column biosampleId to the resulting dataset, indicating valid biosample identifiers.
  • Tests were provided.

@DSuveges DSuveges merged commit 70fd593 into dev Oct 3, 2024
5 checks passed
@DSuveges DSuveges deleted the alegbe_fix_biosample_study_validation branch October 3, 2024 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All GWAS Catalog studies are marked as invalid
2 participants