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

Adding in all the work from the long-term Special Project Malaria branch #440

Merged
merged 15 commits into from
Jan 10, 2024

Conversation

jonn-smith
Copy link
Collaborator

Merging in the long-running branch Kiran and I created for the malaria work.

I have attempted to put everything in reasonable places.

I know this is big, but I'm hoping to merge this fast so I can make a few updates to these pipelines on top of this branch.

Copy link
Collaborator

@bshifaw bshifaw left a comment

Choose a reason for hiding this comment

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

  1. Why are the set of pipelines in a folder called "Z_One_Off_Analyses" not integrated with the rest of the pipeline folder?
  2. In some of the wdls I've commented to move the description to the meta block of the workflow, this is so the wdl doc page on the repository site is formated correctly. The automated documentation is sensitive to hashes at the top of the page.

@@ -137,8 +137,8 @@ task RemovePalindromes {

runtime {
cpu: default_attr.cpu_cores
memory: default_attr.mem_gb + " GiB"
disks: "local-disk " + default_attr.disk_gb + " HDD"
memory: select_first([default_attr.mem_gb]) + " GiB"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are deprecated wdls being updated, are they used by any wdls in the main pipeline folder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated this so that miniwdl check would pass. I was running it on all wdls in the repo before I remembered that the deprecated folder was not being checked. It's not being used anywhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I follow the same logic as Jonn.
That is, I validate locally too, including the deprecated ones. So if any of my changes makes the deprecated ones invalid, I'd do the minimal to make them valid but nothing more.
That being said, I don't fix bugs in those deprecated pipelines or tasks.

wdl/pipelines/TechAgnostic/Utility/ConvertToZarrStore.wdl Outdated Show resolved Hide resolved
@jonn-smith
Copy link
Collaborator Author

@bshifaw The workflows and tasks in the Z_One_Off_Analyses folders are not supposed to be integrated into our main pipelines. They are for one-off analyses that I have needed to make for this work, but that we should not support. There is currently no better place to put them with them still being part of this repo (which they need to be).

Copy link
Collaborator

@bshifaw bshifaw left a comment

Choose a reason for hiding this comment

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

remove the commented description lines at the top from wdl/pipelines/TechAgnostic/Utility/BenchmarkVCFs.wdl

Copy link
Collaborator

@bshifaw bshifaw left a comment

Choose a reason for hiding this comment

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

👍

@jonn-smith jonn-smith merged commit 35a109c into main Jan 10, 2024
5 checks passed
@jonn-smith jonn-smith deleted the jts_malaria_merge branch January 10, 2024 02:19
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.

None yet

4 participants