[Do not merge!] Pseudo PR for first release #28
Conversation
…o isoquant_dev
…r to resolve linting issues
…regardless of if the user chooses to trim files or not
Co-authored-by: Lili Andersson-Li <[email protected]>
Co-authored-by: Lili Andersson-Li <[email protected]>
Co-authored-by: Lili Andersson-Li <[email protected]>
Co-authored-by: Lili Andersson-Li <[email protected]>
Co-authored-by: Lili Andersson-Li <[email protected]>
Co-authored-by: Lili Andersson-Li <[email protected]>
Co-authored-by: Lili Andersson-Li <[email protected]>
| @@ -6,3 +6,8 @@ results/ | |||
| testing/ | |||
| testing* | |||
| *.pyc | |||
| test.sh | |||
| params.yml | |||
There was a problem hiding this comment.
Why do you explicitly ignore params and the samplesheet file?
There was a problem hiding this comment.
During development, we often worked out of the github directory and would place the samplesheet and parameter files in there, so to remove the chance of an accidental commit of either one we went ahead and added them here.
assets/scnanoseq_diagram.png
Outdated
There was a problem hiding this comment.
very nice tube map. small nit-picky thins you can ignore:
- can you please name it something like
scnanoseq_tube_map, so we can pick automatically pick it up for some future nf-core things. - can you also provide an svg version, which makes it easier for collaborators to edit and extend the map
- can you trie to center the "Alignment" and the "Barcode correction" nodes in the middle of their respective curves
- In the legend, I would flip the symbol for File Output 180 degrees (so it ends in the horizontal bar, following the reading direction)
- might be an optical illusion, but are the Count QC nodes correctly horizontally centered between the curves before and after them?
There was a problem hiding this comment.
I went ahead and tried to address all these with commit 86284fa. Let me know if you see any missing/ anything else
docs/usage.md
Outdated
| ``` | ||
|
|
||
| ### Full samplesheet | ||
| `cell_count` refers to the expected number of cells for each sample, which can vary from sample to sample. |
There was a problem hiding this comment.
is that value relevant for a specific processing step?
There was a problem hiding this comment.
It is -- cell count is utilized by the BLAZE barcode caller as a guide to determine the 'correct' number of barcodes that need to be called
There was a problem hiding this comment.
i would clarify that here (or point to the relevant section in the docs)
There was a problem hiding this comment.
I combined this fix with the point below. We have a table describing the columns so I added the expanded description in there (commit 891b67d)
workflows/scnanoseq.nf
Outdated
| blaze_whitelist = file("$baseDir/assets/whitelist/3M-february-2018.zip") | ||
| } | ||
| else{ | ||
| blaze_whitelist = file("$baseDir/assets/whitelist/737K-august-2016.txt.zip") |
There was a problem hiding this comment.
shouldn't then be 10X_5v2 the default value for barcode_format?
There was a problem hiding this comment.
Yeah it does read like that, I think it may make more sense to turn the second else into an else if (params.barcode_format.equals("10X_5v2") for readability, since this param can only be one of those two values anyway so it makes it more clear
There was a problem hiding this comment.
Tried to address this with commit c082e20
…t as a parameter overrides the default whitelist used by the pipeline
|
Hi @mashehu, I believe I managed to address all points, let me know if there's anything missing or if there's any other issues! Thanks, |
|
All good now from my side! 👍🏻 |
|
Awesome, thank you so much! So as far as activating zenodo. Is that just a post in the pipeline-maintainers channel once we merge into the master branch or can that be done before the merge? |
|
zenodo is already activated. so as soon as you make a release, you will get your DOI |
Do not merge! This is a PR of dev compared to first release for whole-pipeline reviewing purposes. Changes should be made to
devand this PR should not be merged into first-commit-for-pseudo-pr!