Conversation
Dev preprocessing
…update channels / dataflow between modules
…formating config file
Major Update tests are failing because test profile hasn't been set up yet
…nel + samplsheet from list with nf-schema
nf-tests for modules
new module and subworkflow test
New functions
updated test profile and docs
There was a problem hiding this comment.
I think I went through everything now.
Good job on writing this pipeline, not an easy one to do.
I left several comments that apply in similar cases as well, please always fix them in all of them.
One major thing that needs to be added imo is some kind of parameter.csv validation. Based on the docs you have already quite a good knowledge of what types and what are the possible values etc. I would write these into a JSON-schema (LLMs are quite good at that) and then have a process like this to validate the csv files:
process VALIDATE_PARAMETERS {
tag "$meta.id"
input:
tuple val(meta), path(parameters_csv)
path schema_json
output:
tuple val(meta), path(parameters_csv), emit: validated
script:
"""
#!/usr/bin/env python3
import json
import csv
import sys
from jsonschema import validate, ValidationError
# Load schema
with open('${schema_json}') as f:
schema = json.load(f)
# Read CSV and convert to dict/list for validation
with open('${parameters_csv}') as f:
reader = csv.DictReader(f)
data = list(reader)
# Validate
try:
validate(instance=data, schema=schema)
print(f"✓ Validation passed for ${meta.id}")
except ValidationError as e:
print(f"✗ Validation failed for ${meta.id}: {e.message}")
sys.exit(1)
"""
}| nf_core_version: 3.5.1 | ||
| repository_type: pipeline | ||
| lint: | ||
| included_configs: False |
There was a problem hiding this comment.
is this really needed?
There was a problem hiding this comment.
i get a linting error if i don't have this .... i didn't know to solve it otherwise
|
|
||
| **Numorph3DUnet** performs cell-nuclei segmentation and quantification from the nuclear channel. | ||
|
|
||
| ### Mat2JSON |
There was a problem hiding this comment.
this feels like a generic module and not a tool. does it really generate output in the results folder?
There was a problem hiding this comment.
Yes, for some output files that are in .mat format, it converts them into JSON or CSV. Because .mat files can only be opened by MATLAB
| assertAll( | ||
| { assert snapshot( | ||
| // Number of successful tasks | ||
| workflow.trace.succeeded().size(), |
There was a problem hiding this comment.
same as before. for subworkflows caputring the process outputs is preferred
There was a problem hiding this comment.
i can change that , but i saw in the documentation now that for subworkflows one should check with workflow. and i think it would make sense to do so.
https://nf-co.re/docs/tutorials/tests_and_test_data/components/04_testing_subworkflows
therefore i think i would leave it like that. Or maybe i misunderstood something
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Adding review suggestions
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Adding Review feedback
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
adding review suggestions
update ara subworkflow
Do not merge! This is a PR of
devcompared to theTEMPLATEbranch for whole-pipeline reviewing purposes. Changes should be made todevand this PR should not be merged!