Skip to content

Add a multiqc report and containerize the trimming - #12

Merged
cokelaer merged 2 commits into
sequana:mainfrom
cokelaer:main
Aug 24, 2026
Merged

Add a multiqc report and containerize the trimming#12
cokelaer merged 2 commits into
sequana:mainfrom
cokelaer:main

Conversation

@cokelaer

Copy link
Copy Markdown
Contributor

Follow-up to #11, whose merge did not include this commit.

MultiQC

A multiqc rule aggregates the FastQC and cutadapt results into
multiqc/multiqc_report.html, which each sample report links from its header.
It uses the multiqc/run shell of sequana-wrappers and the multiqc container,
as the other pipelines do. A multiqc section was added to config.yaml and
to the schema, and multiqc was declared in tools.txt and environment.yml.

Containerized trimming

The trimming used to assemble its command line inside a python run:
directive, which cannot be executed in a container. It is now a plain shell:
rule using the cutadapt container (cutadapt 4.6), with single-end and
paired-end variants, and the declared threads are passed to cutadapt -j.

The atropos variant is kept as a separate rule. No container exists for
atropos, so it must be available locally. Its own json/txt report is no longer
overwritten by the standard output.

Logs

fastq_stat_samples, fastq_stat_phix, fastq_stat_trimmed and
bwa_bam_to_fastq now have a log: file, written as the rule progresses so
that the pipeline monitor reports the real elapsed time instead of a few
seconds. Those four rules stay in python: they call the sequana API, which the
report generation requires on the host anyway. summary is now a local rule.

Also requires sequana >= 0.24.1. Version 1.3.0.

Validation

  • dry run of four option combinations: 17, 10, 14 and 7 jobs, no error
  • full run without container: exit 0, multiqc report and summary.html created,
    all log files written
  • full run with --apptainer-prefix: exit 0, and the log shows the container
    was used (This is cutadapt 4.6 versus cutadapt 5.2 on the host)
  • pytest test/test_main.py: 8 passed

🤖 Generated with Claude Code

MultiQC aggregates the FastQC and cutadapt results in multiqc/multiqc_report.html
and each sample report links it. The rule uses the multiqc container and the
sequana_wrappers shell, as in the other pipelines.

The trimming used to build its command line inside a python 'run' directive,
which cannot be executed in a container. It is now a plain shell rule using
the cutadapt container (cutadapt 4.6). The atropos variant is kept as a
separate rule; atropos has no container so it must be available locally. Its
own report is no longer overwritten by the standard output.

The rules that remain in python (the fastq statistics and the extraction of
the mapped reads) now have a log file, which they write as they go: the
pipeline monitor follows the modification times to report the elapsed time.
Those rules use the sequana library directly, which the report generation
requires on the host anyway, hence no container for them.

Also require sequana >=0.24.1, declare multiqc in tools.txt and in the conda
environment, and run the summary rule locally.
@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds MultiQC aggregation and improves portability/observability of the QC pipeline by running cutadapt trimming in a container and writing progress logs for long-running Python rules.

Changes:

  • Add a multiqc Snakemake rule producing multiqc/multiqc_report.html, plus config/schema/tooling declarations and a link from sample reports.
  • Refactor trimming into containerized shell: cutadapt rules (single/paired) while keeping an atropos fallback rule.
  • Add log: files to several Python run: rules and make summary a local rule; bump version to 1.3.0 and require sequana >= 0.24.1.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
sequana_pipelines/quality_control/tools.txt Declares multiqc as a pipeline tool.
sequana_pipelines/quality_control/schema.yaml Adds multiqc config validation schema.
sequana_pipelines/quality_control/quality_control.rules Implements MultiQC rule/linking, containerized cutadapt trimming, and adds progress logs/localrules.
sequana_pipelines/quality_control/config.yaml Adds cutadapt/multiqc apptainer images and new multiqc config section.
README.rst Documents new 1.3.0 features/requirements in the changelog.
pyproject.toml Bumps version, adds keyword, and raises sequana minimum version.
environment.yml Adds multiqc to the environment dependencies.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +423 to +427
output:
fastq=__cutadapt__output
params:
options=trimming_options,
report="{sample}/logs/cutadapt/cutadapt"
Comment on lines 51 to +54
pylab.ioff()
for filename, json_file, gc_file, boxplot_file in zip(fastqs, output.json, output.gc, output.boxplot):
with open(logfile, "a") as fout:
fout.write(f"Computing the statistics of {filename}\n")
Comment on lines +402 to +406
output:
fastq=__cutadapt__output
params:
options=trimming_options,
report="{sample}/logs/cutadapt/cutadapt"
With atropos, the cutadapt.txt report was created as a side effect of
--report-file. Nothing declared it, so multiqc, which reads that file,
could not be resolved and the DAG could not be built:

    MissingInputException in rule multiqc:
        affected files: data/logs/cutadapt/cutadapt.txt

The report is now declared in the log section of the atropos rules, next
to the standard output. A test builds the workflow with atropos.

The statistics rules also truncate their log file before writing, so a
rerun no longer appends to the messages of the previous run.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

sequana_pipelines/quality_control/quality_control.rules:106

  • multiqc is a new optional config section, but the workflow indexes it with config['multiqc'], which raises a KeyError when running with an older/minimal config that doesn’t define multiqc. Use config.get('multiqc', {}) (or manager.config.multiqc) so the pipeline can still run when the section is absent and MultiQC is effectively disabled.

This issue also appears in the following locations of the same file:

  • line 487
  • line 494
  • line 644
if config['multiqc']['do']:
    expected_output += ["multiqc/multiqc_report.html"]

sequana_pipelines/quality_control/quality_control.rules:488

  • This if config['multiqc']['do']: guard assumes the multiqc section always exists. If a config file omits the multiqc block, the workflow fails at parse time with KeyError before any rules run. Consider guarding with config.get('multiqc', {}).get('do', False) so MultiQC is simply skipped when not configured.
if config['multiqc']['do']:

sequana_pipelines/quality_control/quality_control.rules:646

  • The MultiQC link injection also assumes config['multiqc'] is present. With an older/minimal config file, report generation will crash with KeyError even if MultiQC isn’t intended to run. Guard this with config.get('multiqc', {}).get('do', False) (or a shared multiqc_enabled flag).
        multiqc_link = ''
        if config['multiqc']['do']:
            multiqc_link = ' | <a href="../../multiqc/multiqc_report.html">MultiQC</a>'

sequana_pipelines/quality_control/quality_control.rules:498

  • In the multiqc rule, the schema marks several keys under multiqc as optional (required: False), but the rule accesses them via config['multiqc']['...']. If a user removes one of these optional keys from their config, the workflow will raise a KeyError at parse time. Using .get(..., default) avoids hard failures and keeps the config’s optionality real.
        params:
            options=config['multiqc']['options'],
            input_directory=config['multiqc']['input_directory'],
            config_file=config['multiqc']['config_file'],
            modules=config['multiqc']['modules']

@cokelaer
cokelaer merged commit 25b36d3 into sequana:main Aug 24, 2026
2 of 3 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.

2 participants