Skip to content

Release PR 1.0.0#27

Merged
nvnieuwk merged 183 commits into
masterfrom
dev
Mar 26, 2026
Merged

Release PR 1.0.0#27
nvnieuwk merged 183 commits into
masterfrom
dev

Conversation

@nvnieuwk

Copy link
Copy Markdown
Member

First release of the pipeline

Tuur-ds and others added 30 commits April 30, 2025 12:53
Added and integrated the samtools/view module, updated the sampleshee…
Added position specifier for samtools/view, added and integrated the …
Added samtools/fastq module and integrated it
Added and integrated pear module
Co-authored-by: Nicolas Vannieuwkerke <101190534+nvnieuwk@users.noreply.github.com>
Added a module to merge reads

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be reference data, or at least live somewhere else? (e.g. outside the pipeline)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch! I forgot to move it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Separate schema's per input?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is one input file per mini pipeline present in the pipeline, I kept it separate because the flows don't interact with eachother and this kept it more clear

Comment thread bin/do_it_gz.sh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a bash script, port it to a module

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ToonRosseel I'll leave this up to you ;)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

addressed in PR #28

Comment thread bin/pacvar_repeat_xlsx_report.py
Comment thread bin/rnafusion_varcov.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the holy hell is this even and why can't this be done with a "normal" tool and a module in multiiqc_cmgg ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's a bit too complex for a multiqc, I tried, this would need a UI on its own which would take more work than we currently have time for

Comment thread conf/igenomes.config

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ditch this and use the config from nf-cmgg/configs or remove it entirely

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Drop custom module, use nf-core CAT_FASTQ or something

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

addressed in PR #28

Comment thread bin/updated_spothot.py
fq_all = f"{outputdir}/{sample}_all.fastq.gz"

### extract reads from BAM/CRAM ###
subprocess.check_output(f"{samtools} view -T {reference_genome} {cram} {pos} -F 12 -b -o {bam}", stderr=subprocess.STDOUT, shell=True)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you kidding me....? pythonbash?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🙂 This is the original script, now replaced by the targeted.nf workflow, and can be removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I removed script in PR #28

Comment thread bin/updated_spothot.py
### check if file is not empty (otherwise pear goes in ERROR) ###
if os.path.getsize(fq_r1) != 0:
### merge overlapping read pairs using pear ###
subprocess.check_output(f"{pear} --forward-fastq {fq_r1} --reverse-fastq {fq_r2} --output {fq_p} >> {log}", stderr=subprocess.STDOUT, shell=True)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we run pear, or any other tool here when we have a module??

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the original script, now replaced by the targeted.nf workflow, and can be removed.

@matthdsm matthdsm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't really like adding all the crap to the history, but whatever I guess.
Let's just do this and then deprecate this pipeline ASAP entirely

@nvnieuwk nvnieuwk merged commit 3fd6ac7 into master Mar 26, 2026
40 of 41 checks passed
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.

5 participants