Skip to content

Remove outdated code and documentation related to evidence assessment#859

Open
epiercehoffman wants to merge 2 commits intomainfrom
eph_02_cleanout
Open

Remove outdated code and documentation related to evidence assessment#859
epiercehoffman wants to merge 2 commits intomainfrom
eph_02_cleanout

Conversation

@epiercehoffman
Copy link
Collaborator

@epiercehoffman epiercehoffman commented Aug 22, 2025

Updates

  • Remove outdated code and documentation related to evidence assessment (GenerateBatchMetrics): there were many scripts under src/sv-pipeline/02_evidence_assessment/ that are no longer used (now use svtk, GATK). The accompanying READMEs are misleading and create clutter. I propose that we do not need to move these to gatk-sv-internal as they are already in the private gatk-sv-v1 repo and have been outdated for several years
  • Remove additional outdated or unhelpful subdirectory READMEs
  • Move src/sv-pipeline/02_evidence_assessment/estimated_CN_denoising.py to src/sv-pipeline/pre_SVCalling_and_QC/ as it is part of EvidenceQc rather than GenerateBatchMetrics

Testing

  • Read each README and verified they were outdated/misleading
  • Searched for each script in the repo and verified it was not in use
  • Built sv-pipeline docker and successfully ran GenerateBatchMetrics & EvidenceQc with it

Notes

  • Before merging, changes to dockstore.yml need to be reverted
  • The docker build for sv-shell is failing but it seems to be unrelated to my changes, as it also fails when I try to build it on the main branch

@VJalili
Copy link
Member

VJalili commented Aug 26, 2025

Thanks for the clean up @epiercehoffman

The docker build for sv-shell is failing but it seems to be unrelated to my changes, as it also fails when I try to build it on the main branch

Correct, the failing Docker build seems unrelated to this PR, and reproducible on the latest main. For context, the error complains about installing R packages and the failing part is common between sv-shell and scramble docker images. The error is mostly likely cran-related as the last build of gatk-sv-bot was successful.

If you want to update the Docker images when this PR is merged, this error needs to be fixed separately and merged before merging this PR, otherwise, the bot will fail to update the Docker images list.

@epiercehoffman
Copy link
Collaborator Author

Thanks for looking into the docker build issues @VJalili!

If you want to update the Docker images when this PR is merged, this error needs to be fixed separately and merged before merging this PR, otherwise, the bot will fail to update the Docker images list.

Thanks for this note - we will want to make sure that the dockers get updated with this PR given that I moved one script so the WDLs and dockers would be out of sync otherwise. This PR isn't urgent, so it can wait for the docker build fix.

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