Add fqtk as a demultiplexer #99
Merged
Conversation
This file contains 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
|
|
I don't mind them being squashed but that commit message 1. Has nothing to do with fqtk 2. Is from another PR and I think we from a rebase that didn't work out as planned. |
Co-authored-by: ewels <[email protected]> This is a combination of 3 commits. Fqtk off of dev Remove todo's from fqtk_demultiplex.nf Update demultiplex.nf Update README.md Update input file paths
Update CHANGELOG.md
edmundmiller
requested changes
Mar 15, 2023
|
|
||
| rg.ID = [fcid,lane].join(".") | ||
| rg.PU = [fcid, lane, index].findAll().join(".") | ||
| rg.PL = "SINGULAR" |
There was a problem hiding this comment.
Should this be hard-coded?
There was a problem hiding this comment.
@emiller88 Good point, it probably should not be hardcoded. How else could it be filled in?
There was a problem hiding this comment.
There might be a way to read in the read group. We can just throw a TODO on it though for now.
Comment on lines
+390
to
+423
| def extract_csv_fqtk(input_csv) { | ||
|
|
||
| // Flowcell Sheet schema | ||
| // Possible values for the "content" column: [meta, path, number, string, bool] | ||
| def input_schema = [ | ||
| 'columns': [ | ||
| 'id': [ | ||
| 'content': 'meta', | ||
| 'meta_name': 'id', | ||
| 'pattern': '', | ||
| ], | ||
| 'samplesheet': [ | ||
| 'content': 'path', | ||
| 'pattern': '^.*.csv$', | ||
| ], | ||
| 'lane': [ | ||
| 'content': 'meta', | ||
| 'meta_name': 'lane', | ||
| 'pattern': '', | ||
| ], | ||
| 'flowcell': [ | ||
| 'content': 'path', | ||
| 'pattern': '', | ||
| ], | ||
| 'per_flowcell_manifest': [ | ||
| 'content': 'path', | ||
| 'pattern': '', | ||
| ] | ||
| ], | ||
| required: ['id','flowcell', 'samplesheet', 'per_flowcell_manifest'], | ||
| ] | ||
|
|
||
| return extract_csv(input_csv, input_schema) | ||
| } |
There was a problem hiding this comment.
These are starting to get excessive. @matthdsm what happens if we put them in lib/? Would they still work the same?
There was a problem hiding this comment.
@emiller88 Moving the prep for ch_flowcells into subworkflows would remove the need for these functions to be in demultiplex.nf, right?
There was a problem hiding this comment.
Right, that's a great idea!
There was a problem hiding this comment.
I've no qualms putting all the functions in /lib. They're only in the main file for convenience!
Co-authored-by: Edmund Miller <[email protected]>
sam-white04
commented
Mar 15, 2023
There was a problem hiding this comment.
@emiller88 Thank you for your comments. If everyone is on the same page, Im happy to pull the generation of ch_flowcells and ch_flowcells_tar into subworkflows for fqtk vs the other demultiplexers.
|
|
||
| rg.ID = [fcid,lane].join(".") | ||
| rg.PU = [fcid, lane, index].findAll().join(".") | ||
| rg.PL = "SINGULAR" |
There was a problem hiding this comment.
@emiller88 Good point, it probably should not be hardcoded. How else could it be filled in?
Comment on lines
+390
to
+423
| def extract_csv_fqtk(input_csv) { | ||
|
|
||
| // Flowcell Sheet schema | ||
| // Possible values for the "content" column: [meta, path, number, string, bool] | ||
| def input_schema = [ | ||
| 'columns': [ | ||
| 'id': [ | ||
| 'content': 'meta', | ||
| 'meta_name': 'id', | ||
| 'pattern': '', | ||
| ], | ||
| 'samplesheet': [ | ||
| 'content': 'path', | ||
| 'pattern': '^.*.csv$', | ||
| ], | ||
| 'lane': [ | ||
| 'content': 'meta', | ||
| 'meta_name': 'lane', | ||
| 'pattern': '', | ||
| ], | ||
| 'flowcell': [ | ||
| 'content': 'path', | ||
| 'pattern': '', | ||
| ], | ||
| 'per_flowcell_manifest': [ | ||
| 'content': 'path', | ||
| 'pattern': '', | ||
| ] | ||
| ], | ||
| required: ['id','flowcell', 'samplesheet', 'per_flowcell_manifest'], | ||
| ] | ||
|
|
||
| return extract_csv(input_csv, input_schema) | ||
| } |
There was a problem hiding this comment.
@emiller88 Moving the prep for ch_flowcells into subworkflows would remove the need for these functions to be in demultiplex.nf, right?
edmundmiller
approved these changes
Mar 15, 2023
There was a problem hiding this comment.
I think this looks good to me. #102 For follow up to clean up some things that needed to be introduced here.
matthdsm
approved these changes
Mar 16, 2023
|
If you rerun the CI enough, it works 🙃 |
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.
Hello,
I have added fqtk as an optional demultiplexer. The tool fqtk requires an additional input to be provided as a path in the 5th column of
--input samplesheet.csv.Thank you,
Samantha White
PR checklist
nf-core lint).nextflow run . -profile test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated. (@sam-white04 Will update as soon as PR is posted)README.mdis updated (including new tool citations and authors/contributors).