Conversation
Reviewer's GuideIntroduce a multi-stage publication-style pileup pipeline that separates raw, modified, and color-coded outputs, driven by new R and Python scripts, with corresponding workflow wiring, configuration options, documentation, and tests, plus a small CLI bug fix in merge_tables.R. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deliaBlue
marked this pull request as ready for review
June 27, 2026 11:54
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 12 issues, and left some high level feedback:
- The new pileup modification rules in
pileup.smk(modify_per_library/run/condition_ascii_pileups) share a lot of duplicated parameters and shell command structure; consider factoring the common logic into a single rule with wildcards or a reusable wrapper to reduce maintenance overhead. - There are several naming inconsistencies between file prefixes and documentation (e.g.
all-samplesvsall_samples,group-Avsgroup_A,lib-namevslib_name); aligning these across the workflow, scripts, and output examples would make the pipeline easier to follow and less error-prone. - The reference link for the R script in
docs/includes/references.mdpoints toascii_pileup_aesthetics_modification.R, while the actual file name isascii_pileups_aesthetics_modification.R; update the URL (and any similar references) to match the implemented script path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new pileup modification rules in `pileup.smk` (`modify_per_library/run/condition_ascii_pileups`) share a lot of duplicated parameters and shell command structure; consider factoring the common logic into a single rule with wildcards or a reusable wrapper to reduce maintenance overhead.
- There are several naming inconsistencies between file prefixes and documentation (e.g. `all-samples` vs `all_samples`, `group-A` vs `group_A`, `lib-name` vs `lib_name`); aligning these across the workflow, scripts, and output examples would make the pipeline easier to follow and less error-prone.
- The reference link for the R script in `docs/includes/references.md` points to `ascii_pileup_aesthetics_modification.R`, while the actual file name is `ascii_pileups_aesthetics_modification.R`; update the URL (and any similar references) to match the implemented script path.
## Individual Comments
### Comment 1
<location path="workflow/scripts/ascii_pileups_aesthetics_modification.R" line_range="640-641" />
<code_context>
+ header = F,
+ col.names = c( "seq", "counts" ))
+
+ # Store index of the last header line
+ last.header.row <- which(grepl("^.*:", pileup[, 2]))
+
+ # Skip empty pileups unless `--keep-all` is set
</code_context>
<issue_to_address>
**issue (bug_risk):** Use a single header-row index (e.g. max()) instead of the whole which() vector
`which(grepl("^.*:", pileup[, 2]))` returns a vector (one index per header row). Using this vector in `if (!keep.all && nrow(pileup) <= last.header.row)` will error because `if` needs a single logical, and passing it to `SplitArms(head.lines = last.header.row, ...)` also violates the expectation that `head.lines` is scalar. Compute a single index instead, e.g. the last header row:
```r
last.header.row <- max(which(grepl("^.*:", pileup[, 2])))
```
and use this scalar in both the `if` condition and `SplitArms` call.
</issue_to_address>
### Comment 2
<location path="workflow/scripts/ascii_pileups_aesthetics_modification.R" line_range="486-487" />
<code_context>
+#' # FilterPileup(pileup, min.count = 2, max.seq = 20)
+FilterPileup <- function( pileup, min.count, max.seq ) {
+
+ # Store last header line index
+ head.lines <- which(grepl("^.*:", pileup[, 2]))
+
+ # Get aligned reads rows
</code_context>
<issue_to_address>
**issue (bug_risk):** Derive a scalar header index in FilterPileup to avoid slicing on a vector
`which(grepl(...))` can return multiple header rows. Passing that vector into `seq_len()` and `:` implicitly uses only the first element (with warnings), so headers aren’t consistently stripped.
Derive a single scalar index instead, e.g.:
```r
head.lines <- max(which(grepl("^.*:", pileup[, 2])))
```
so `seq_len(head.lines)` and `pileup[1:head.lines, ]` always operate on the full header range.
</issue_to_address>
### Comment 3
<location path="workflow/rules/pileup.smk" line_range="423-429" />
<code_context>
+ PILEUP_DIR / "raw" / "{sample}" / "check_file.txt",
sample=pd.unique(samples_table.index.values),
),
- piles_design=expand(
+ piles_raw_design=expand(
(
- PILEUP_DIR / "{condition}" / "check_file_{condition}.txt"
+ PILEUP_DIR / "raw" / "{cond}" / "check_file_{cond}.txt"
if config["lib_dict"] != None
else []
),
- condition=list(config["lib_dict"].keys()),
+ cond=list(config["lib_dict"].keys()),
),
+ piles_mod_run=PILEUP_DIR / "mod/all/check_file.txt",
</code_context>
<issue_to_address>
**issue:** Guard expand() `cond=list(config["lib_dict"].keys())` when lib_dict can be None
Because `cond=list(config["lib_dict"].keys())` is always evaluated, this code will raise at workflow parsing time if `config["lib_dict"]` is `None`, even though the first argument would resolve to `[]`. To align with the existing guards, you could wrap the `expand()` call itself, e.g.
```python
tiles_design = (
expand(
PILEUP_DIR / "mod" / "{cond}" / "check_file_{cond}.txt",
cond=list(config["lib_dict"].keys()),
)
if config["lib_dict"] is not None
else []
)
```
The same guard should be applied in the `color_code_ascii_pileups` rule to avoid the same failure there when `lib_dict` is intentionally `None`.
</issue_to_address>
### Comment 4
<location path="workflow/Snakefile" line_range="110-116" />
<code_context>
sample=pd.unique(samples_table.index.values),
),
- piles_design=expand(
+ piles_raw_design=expand(
(
- PILEUP_DIR / "{condition}" / "check_file_{condition}.txt"
+ PILEUP_DIR / "raw" / "{cond}" / "check_file_{cond}.txt"
if config["lib_dict"] != None
else []
),
- condition=list(config["lib_dict"].keys()),
+ cond=list(config["lib_dict"].keys()),
),
+ piles_mod_run=PILEUP_DIR / "mod/all/check_file.txt",
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid unconditional access to lib_dict.keys() when lib_dict may be None
`expand()` will still evaluate `list(config['lib_dict'].keys())` even when the template branch would resolve to `[]`, so this will fail if `lib_dict` is `None`.
To handle both `{}` and `None`, guard the whole `expand()` call instead, for example:
```python
piles_raw_design = (
expand(
PILEUP_DIR / 'raw' / '{cond}' / 'check_file_{cond}.txt',
cond=list(config['lib_dict'].keys()),
)
if config['lib_dict'] is not None
else []
)
```
and similarly for `piles_mod_design`.
</issue_to_address>
### Comment 5
<location path="workflow/scripts/ascii_pileups_aesthetics_modification.R" line_range="55" />
<code_context>
+description <- "Enhance raw ASCII-style alignment pileups.\n"
+author <- "Author: Iris Mestres-Pascual <zavolab-biozentrum@unibas.ch>"
+maintainer <- "Maintainer: Iris Mestres-Pascual <zavolab-biozentrum@unibas.ch>"
+version <- "Version: 1.0.0 (ABR-2026)"
+requirements <- "Requires: dplyr, optparse"
+msg <- paste(description, author, maintainer, version, requirements, sep = "\n")
</code_context>
<issue_to_address>
**nitpick (typo):** Fix the month abbreviation in the version string
The month abbreviation in `"Version: 1.0.0 (ABR-2026)"` doesn’t match the English style used elsewhere (e.g. `MAY-2026`). If this is April, please change it to `APR-2026` for consistency.
```suggestion
version <- "Version: 1.0.0 (APR-2026)"
```
</issue_to_address>
### Comment 6
<location path="docs/workflow/modules/pileups.md" line_range="351" />
<code_context>
+ final file (default: 'true')
+ - `color_dict`: Dictionary with the character-to-color mapping. See
+ available colors in the
+ [module overview](../overview.md#ascii-style-alignmnet-pileups-module).
+ (default: 'adenine'='green', 'cytosine'='orange',
+ 'guanine'='"light purple"', 'thymine'='"light blue"',
</code_context>
<issue_to_address>
**issue (typo):** Fix typo in the "ascii-style-alignmnet" anchor text.
The fragment `ascii-style-alignmnet` is misspelled; please change it to `ascii-style-alignment` in the anchor to keep the link consistent.
```suggestion
[module overview](../overview.md#ascii-style-alignment-pileups-module).
```
</issue_to_address>
### Comment 7
<location path="pipeline_documentation.md" line_range="1986-1995" />
<code_context>
+
+=== "Input"
+
+ (**Workflow output**) Empty text file (`.txt`); from
+ [**create_per_library_ascii_pileups**](pileups.md#create_per_library_ascii_pileups)
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Anchor target for the modify_per_library_ascii_pileups link seems incorrect.
The link text is `modify_per_library_ascii_pileups`, but the anchor is `#create_per_library_ascii_pileups`. Update the anchor to `#modify_per_library_ascii_pileups` so the link targets the correct section.
</issue_to_address>
### Comment 8
<location path="README.md" line_range="434-435" />
<code_context>
Finally, to visualize the distribution of read alignments around miRNA
loci, ASCII-style alignment pileups are optionally generated for user-defined
-regions of interest.
+regions of interest. These, are modified and color-coded to produce
+publication-style pileups.
</code_context>
<issue_to_address>
**suggestion (typo):** Remove unnecessary comma after "These".
This should read `These are modified and color-coded to produce publication-style pileups.` (remove the comma after “These”).
```suggestion
Finally, to visualize the distribution of read alignments around miRNA
loci, ASCII-style alignment pileups are optionally generated for user-defined regions of interest. These are modified and color-coded to produce
publication-style pileups.
```
</issue_to_address>
### Comment 9
<location path="README.md" line_range="444-445" />
<code_context>
> identical sequence, or displayed on its own depending on whether alignments
> are collapsed or not.
+The table bellow show the allowed string color names. The text color in each
+cell is the one used in the final representation.
+
</code_context>
<issue_to_address>
**issue (typo):** Fix spelling and verb agreement in "bellow show".
Consider wording it as: `The table below shows the allowed string color names.`
```suggestion
The table below shows the allowed string color names. The text color in each
cell is the one used in the final representation.
```
</issue_to_address>
### Comment 10
<location path="docs/api/ascii_pileups_aesthetics_modification.R.md" line_range="77-79" />
<code_context>
+ independently to each arm.
+- <b>`--min-count INT`</b>: Minimum count for a sequence to be kept
+ (default: 1).
+- <b>`--max-sequences INT`</b>: Maximum number of top sequences to be
+ displayed. It is assumed that the input ASCII-style alignment pileups are
+ alredy sorted in the desired order (default: 30).
+- <b>`--overhang INT`</b>: If `--split-arms` is set, number of extra positions
+ to retain on each side of the mature arm span. Reads extending beyond this
</code_context>
<issue_to_address>
**issue (typo):** Correct "alredy" to "already".
In the sentence about ASCII-style alignment pileups, change `alredy` to `already`.
```suggestion
- <b>`--max-sequences INT`</b>: Maximum number of top sequences to be
displayed. It is assumed that the input ASCII-style alignment pileups are
already sorted in the desired order (default: 30).
```
</issue_to_address>
### Comment 11
<location path="docs/api/copper.py.md" line_range="13" />
<code_context>
+If a directory is provided as input, an HTML file is generated for each
+file with just one CSS file.
+
+For a proper HTML creation, the counts column title must be specified in
+the CLI argument `--counts_id` (see the "Constraints" section for a more
+detailed explanation on the input format)
</code_context>
<issue_to_address>
**suggestion (typo):** Slightly improve phrasing of "For a proper HTML creation".
Consider dropping the article: `For proper HTML creation, the counts column title must be specified in the CLI argument ...`.
```suggestion
For proper HTML creation, the counts column title must be specified in
```
</issue_to_address>
### Comment 12
<location path="docs/api/copper.py.md" line_range="272-273" />
<code_context>
+Rendered HTML fragment and current sequence length.
+
+- <b>`html`</b>: Row HTML string representation.
+- <b>`seq_len`</b>: Lenght of the sequence representation
+
+---
</code_context>
<issue_to_address>
**issue (typo):** Fix spelling of "Lenght" to "Length".
```suggestion
- <b>`html`</b>: Row HTML string representation.
- <b>`seq_len`</b>: Length of the sequence representation
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Collaborator
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The default
color_dictvalues inconfig_template.yamlforguanine,thymine, andgapinclude extra double quotes (e.g."light purple"), which will not match the allowedcopper.pycolor choices and likely cause argument parsing errors; these should be plain strings without embedded quotes. - The
custom-script-ascii-modlink indocs/includes/references.mdpoints toascii_pileup_aesthetics_modification.R, but the actual script path isascii_pileups_aesthetics_modification.R, so the reference should be updated for consistency and to avoid dead links.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The default `color_dict` values in `config_template.yaml` for `guanine`, `thymine`, and `gap` include extra double quotes (e.g. `"light purple"`), which will not match the allowed `copper.py` color choices and likely cause argument parsing errors; these should be plain strings without embedded quotes.
- The `custom-script-ascii-mod` link in `docs/includes/references.md` points to `ascii_pileup_aesthetics_modification.R`, but the actual script path is `ascii_pileups_aesthetics_modification.R`, so the reference should be updated for consistency and to avoid dead links.
## Individual Comments
### Comment 1
<location path="workflow/scripts/copper.py" line_range="298" />
<code_context>
+
+ html_pileup = f"""
+ <div class="line">
+ <div class"line-sequence">
+ {self.get_char_seq(seq, shift)}
+ <span class="line-data">{vals}<strong>{info}</strong></span>
</code_context>
<issue_to_address>
**issue (bug_risk):** All HTML row renderers use `class"line-sequence"` (missing `=`), producing invalid markup.
In `_format_one_field_row`, `_format_two_field_row`, and `_format_three_field_row`, the markup uses `<div class"line-sequence">` instead of `<div class="line-sequence">`, making the `class` attribute invalid and breaking `.line-sequence` styling. Please update all three helpers to emit the correct `class="line-sequence"` attribute.
</issue_to_address>
### Comment 2
<location path="pipeline_documentation.md" line_range="1978-1987" />
<code_context>
+
+### `modify_per_library_ascii_pileups`
+
+Modify the generated ASCII-style pileups for all the desired annotated regions
+across libraries with a [**custom script**][custom-script-ascii-mod].
+
</code_context>
<issue_to_address>
**issue (typo):** Typo: "rune" should be "run"
This sentence should say "for the whole run", not "for the whole rune".
Suggested implementation:
```
+### `modify_per_library_ascii_pileups`
+
+Modify the generated ASCII-style pileups for all the desired annotated regions
+across libraries with a [**custom script**][custom-script-ascii-mod] for the whole run.
```
If "for the whole rune" appears elsewhere in `pipeline_documentation.md`, those occurrences should also be updated to "for the whole run" using the same pattern.
</issue_to_address>
### Comment 3
<location path="docs/api/ascii_pileups_aesthetics_modification.R.md" line_range="272" />
<code_context>
+
+- <b>`in.pileup`</b>: A data frame representing one input pileup. It must
+ contain the columns `seq` and `counts`.
+- <b<`overhang`</b>: A non-negative integer specifying the allowed extension
+ beyond the arm span on each side. If `NULL`, it is treated as `0`.
+- <b>`head.lines`</b>: An integer giving the number of header lines in the
</code_context>
<issue_to_address>
**issue (typo):** Typo in `<b>` tag for `overhang` argument
This line uses `<b<` instead of `<b>`, which breaks the HTML; update it to `<b>` to match the other argument descriptions.
```suggestion
- <b>`overhang`</b>: A non-negative integer specifying the allowed extension
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
uniqueg
approved these changes
Jul 16, 2026
Member
There was a problem hiding this comment.
Thanks! I'm approving this (pending GitHub Copilot and Sourcery comments), because I'm sure it works and does the job just fine, and that's what we need right now :)
But please create an issue to improve the repo structure to make it more maintainable. There is too much repetition, too many different manually generated/curated documentation layers with different levels of detail and hardcoded stuff, making it extremely (and increasingly) difficult to make even simple changes (not that this one was simple, but it shouldn't need changes to 10+ doc files).
| "keep_all": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Write the pileup even if it as no aligned sequences." |
Member
There was a problem hiding this comment.
Not quite clear to me what this does.
| "canonical": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Mark the aligned read corresponding to the canonical mature sequence." |
Member
There was a problem hiding this comment.
Not quite clear to me what this does.
| "split": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Split precursor pileups into one mature-arm pileup per arm." |
Member
There was a problem hiding this comment.
Not quite clear to me what this does.
| # Dictionary with the character-to-color-mapping. See available colors in the | ||
| # main README. | ||
| # | ||
| # Keys must not be changed! |
Member
There was a problem hiding this comment.
Wouldn't it then be easier to use an ordered list?
|
|
||
| Enhance ASCII-style alignment pileups. | ||
|
|
||
| Filter each ASCII-style alignment pileup in the provided input directory |
Member
There was a problem hiding this comment.
This sounds like the input is assumed to be sorted by counts, in descending order. Correct? If so, it might be good to mention that. It needs to be clear that up to max_sequences sequences with the highest counts are retained, as long as their counts are >= min_count. If that's indeed the desired and implemented behavior.
| adjusted to the final representation. If no overhang is provided, only reads | ||
| fully contained within the exact arm span are kept. | ||
|
|
||
| If `--keep-all` is set, the ASCII-style alignment pileup is written even if it |
Member
There was a problem hiding this comment.
So this specifically refers to "empty" pileups, right? So perhaps --keep-empty would be more descriptive/specific?
|
|
||
|
|
||
|
|
||
| The ASCII-style alignment representation is expected to be one of |
Member
There was a problem hiding this comment.
This is hard even for me to imagine. I think an example would help.
Actually, maybe examples would be good for every option...
Member
There was a problem hiding this comment.
This contains a whole lot of duplicate information, which makes it very hard to maintain this. Isn't it possible to auto-generate these docs from the CLI config/code so that we don't need to repeat ourselves? Typically, API docs aren't produced manually, the maintenance burden and error risk is too high.
Member
There was a problem hiding this comment.
This file also contains a lot of manually generated duplicate content. Maybe we can pull this from the code/config?
There was a problem hiding this comment.
Pull request overview
This PR implements a multi-stage “publication-style pileups” pipeline: it restructures pileup outputs into raw → modified → color-coded HTML tiers, adds the required R/Python tooling to transform pileups, and updates workflow wiring, documentation, and integration expectations accordingly. It also fixes merge_tables.R --prefix parsing (Conda integration).
Changes:
- Restructure pileup outputs into
results/pileups/{raw,mod,color_coded}/...and wire new stages into Snakemake finish targets. - Add
ascii_pileups_aesthetics_modification.R(R) to filter/split/annotate canonical reads andcopper.py(Python) to render color-coded HTML + CSS, including new unit tests + fixtures. - Update configs, docs, API pages, and integration-test expected outputs to reflect the new pileup stages and artifacts.
Reviewed changes
Copilot reviewed 24 out of 29 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| workflow/Snakefile | Updates top-level finish targets to expect raw/mod/color-coded pileup check files. |
| workflow/rules/pileup.smk | Refactors pileup rules to emit raw/, adds mod/ R-based modification rules, and adds color_coded/ HTML generation rule. |
| workflow/scripts/copper.py | New Python CLI + renderer for converting modified ASCII pileups into color-coded HTML and shared CSS. |
| workflow/scripts/tests/test_copper.py | New unit tests covering copper CLI parsing, rendering, and directory handling. |
| workflow/scripts/tests/files/pileups/shift/test-lib.hsa-mir-520a.3-shift.tab | New fixture pileup input for shifted precursor-style data. |
| workflow/scripts/tests/files/pileups/shift/test-lib.hsa-miR-520a-3p.5-shift.tab | New fixture pileup input for shifted mature-arm data. |
| workflow/scripts/tests/files/pileups/no_shift/test-lib.hsa-mir-520a.0-shift.tab | New fixture pileup input for non-shifted precursor-style data. |
| workflow/scripts/tests/files/pileups/no_shift/test-lib.hsa-miR-516a-3p.0-shift.tab | New fixture pileup input for empty/non-populated output cases. |
| workflow/scripts/tests/files/pileups/no_shift/test-lib.hsa-miR-1323.0-shift.tab | New fixture pileup input for non-shifted mature-arm data. |
| workflow/scripts/merge_tables.R | Fixes --prefix to store a string value and bumps script version metadata. |
| workflow/scripts/ascii_pileups_aesthetics_modification.R | New R CLI/script to filter pileups, optionally split arms, adjust coordinates, and mark canonical reads. |
| test/test_integration_workflow/test_workflow_local_with_conda.sh | Enables --show-failed-logs to improve debugging on failure. |
| test/test_integration_workflow/test_workflow_local_with_apptainer.sh | Enables --show-failed-logs to improve debugging on failure. |
| test/test_integration_workflow/expected_output.md5 | Updates expected artifact set for new pileup directory tiers and outputs. |
| README.md | Documents publication-style pileups and links an “allowed colors” asset. |
| pipeline_documentation.md | Extends pipeline docs to cover new modify/color-code rules and their parameters/outputs. |
| docs/workflow/overview.md | Adds overview content and images for color-coded pileups and customization tip. |
| docs/workflow/modules/pileups.md | Updates module docs to include modify + color-coding stages and configuration parameters. |
| docs/includes/references.md | Adds references for new custom scripts and pileup format link. |
| docs/guides/outputs.md | Updates expected output tree to include raw/, mod/, and color_coded/ pileup outputs. |
| docs/api/copper.py.md | New generated API reference page for copper.py. |
| docs/api/ascii_pileups_aesthetics_modification.R.md | New generated API reference page for the new R script. |
| config/config_template.yaml | Adds pileup modification parameters and a nucleotide→color mapping configuration block. |
| config/config_schema.json | Extends schema with new pileup modification and color-coding configuration fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+302
to
+318
| split_arms=lambda wc: "--split-arms" if config["split"] else "", | ||
| canonical=lambda wc: "--canonical" if config["canonical"] else "", | ||
| keep_all=lambda wc: "--keep-all" if config["keep_all"] else "", | ||
| min_count=config["min_count_dict"]["lib"], | ||
| max_seq=config["max_seq"], | ||
| overhang=config["extension"], | ||
| shell: | ||
| "(touch {output.piles} && Rscript {input.script} \ | ||
| --verbose \ | ||
| --in-dir={params.in_dir} \ | ||
| --prefix={params.prefix} \ | ||
| --out-dir {params.out_dir} \ | ||
| --min-count {params.min_count} \ | ||
| --max-sequences {params.max_seq} \ | ||
| --overhang {params.overhang} \ | ||
| {params.split_arms} {params.canonical} {params.keep_all} \ | ||
| ) &> {log}" |
Comment on lines
+345
to
+361
| split_arms=lambda wc: "--split-arms" if config["split"] else "", | ||
| canonical=lambda wc: "--canonical" if config["canonical"] else "", | ||
| keep_all=lambda wc: "--keep-all" if config["keep_all"] else "", | ||
| min_count=config["min_count_dict"]["run"], | ||
| max_seq=config["max_seq"], | ||
| overhang=config["extension"], | ||
| shell: | ||
| "(touch {output.piles} && Rscript {input.script} \ | ||
| --verbose \ | ||
| --in-dir={params.in_dir} \ | ||
| --prefix={params.prefix} \ | ||
| --out-dir {params.out_dir} \ | ||
| --min-count {params.min_count} \ | ||
| --max-sequences {params.max_seq} \ | ||
| --overhang {params.overhang} \ | ||
| {params.split_arms} {params.canonical} {params.keep_all} \ | ||
| ) &> {log}" |
Comment on lines
+392
to
+408
| split_arms=lambda wc: "--split-arms" if config["split"] else "", | ||
| canonical=lambda wc: "--canonical" if config["canonical"] else "", | ||
| keep_all=lambda wc: "--keep-all" if config["keep_all"] else "", | ||
| min_count=config["min_count_dict"]["condition"], | ||
| max_seq=config["max_seq"], | ||
| overhang=config["extension"], | ||
| shell: | ||
| "(touch {output.piles} && Rscript {input.script} \ | ||
| --verbose \ | ||
| --in-dir={params.in_dir} \ | ||
| --prefix={params.prefix} \ | ||
| --out-dir {params.out_dir} \ | ||
| --min-count {params.min_count} \ | ||
| --max-sequences {params.max_seq} \ | ||
| --overhang {params.overhang} \ | ||
| {params.split_arms} {params.canonical} {params.keep_all} \ | ||
| ) &> {log}" |
Comment on lines
+493
to
+497
| read.rows <- read.rows %>% | ||
| # Keep alignments with at least `min.count` | ||
| dplyr::filter( as.numeric(counts) >= min.count ) %>% | ||
| # Keep top `max.seq` rows | ||
| dplyr::slice( 1: max.seq ) |
| [code-oligomap]: <https://github.com/zavolanlab/oligomap> | ||
| [code-samtools]: <https://github.com/samtools/samtools> | ||
| [conda]: <https://docs.conda.io/projects/conda/en/latest/index.html> | ||
| [custom-script-ascii-mod]: <https://github.com/zavolanlab/mirflowz/blob/dev/workflow/scripts/ascii_pileup_aesthetics_modification.R> |
| "keep_all": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Write the pileup even if it as no aligned sequences." |
Comment on lines
+240
to
+248
| .char-box { | ||
| display: inline-block; | ||
| width: 15px; | ||
| height: 15px; | ||
| margin-right: 1px; | ||
| line-height: 15px; | ||
| text-align: center; | ||
| font-family: serif, Courier New; | ||
| } |
Comment on lines
+88
to
+93
| "--min-count", | ||
| action = "store", | ||
| type = "numeric", | ||
| default = 1, | ||
| help = "Minimum count for a sequence to be kept. [default %default]", | ||
| metavar = "int" |
Comment on lines
+319
to
+325
| html_pileup = f""" | ||
| <div class="line"> | ||
| <div class="line-sequence"> | ||
| {self.get_char_seq(seq, shift)} | ||
| <span class="line-data">{vals}</span> | ||
| </div> | ||
| </div>\n |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #149.
In addition, the parsing of the parameter
--prefixin themerge_tables.Rscript now has the correct action type. This fixes the integration test via Conda.Conventional Commits
Conventional Commits specification
Checklist
works
reduced the code coverage relative to the previous state
affected by the proposed changes
Summary by Sourcery
Add a multi-stage pipeline to transform raw ASCII-style pileups into modified and publication-ready color-coded HTML outputs, and update configs, docs, tests, and scripts accordingly.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: