Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Longread coverage workflow #82

Merged
merged 9 commits into from
Apr 19, 2023
Merged

Longread coverage workflow #82

merged 9 commits into from
Apr 19, 2023

Conversation

weaglesBio
Copy link
Contributor

Added long read coverage workflow and required modules for it.

PR checklist

  • This comment contains a description of changes (with reason).
  • [] If you've fixed a bug or added code that should be tested, add tests!
    • [] If you've added a new tool - have you followed the pipeline conventions in the contribution docs
    • [] If necessary, also make a PR on the nf-core/treeval branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • [] Usage Documentation in docs/usage.md is updated.
  • [] Output Documentation in docs/output.md is updated.
  • [] CHANGELOG.md is updated.
  • [] README.md is updated (including new tool citations and authors/contributors).

@weaglesBio weaglesBio added the enhancement New feature or request label Mar 30, 2023
@weaglesBio weaglesBio linked an issue Mar 30, 2023 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Mar 30, 2023

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 38e33d6

+| ✅ 134 tests passed       |+
#| ❔  15 tests were ignored |#
!| ❗  13 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: assets/multiqc_config.yml
  • files_exist - File not found: pyproject.toml
  • pipeline_todos - TODO string in README.md: Add full-sized test dataset and amend the paragraph below if applicable
  • pipeline_todos - TODO string in README.md: Update the example "typical command" below used to run the pipeline
  • pipeline_todos - TODO string in WorkflowMain.groovy: Add Zenodo DOI for pipeline after first release
  • pipeline_todos - TODO string in test_full.config: Specify the paths to your full test data ( on nf-core/test-datasets or directly in repositories, e.g. SRA)
  • pipeline_todos - TODO string in test_full.config: Give any required params for the test so that command line flags are not needed
  • pipeline_todos - TODO string in test.config: Specify the paths to your full test data ( on nf-core/test-datasets or directly in repositories, e.g. SRA)
  • pipeline_todos - TODO string in test.config: Give any required params for the test so that command line flags are not needed
  • pipeline_todos - TODO string in base.config: Customise requirements for specific processes.
  • pipeline_todos - TODO string in awsfulltest.yml: You can customise AWS full pipeline tests as required
  • pipeline_todos - TODO string in usage.md: Add documentation about anything specific to running your pipeline. For general topics, please point to (and add to) the main nf-core website.
  • pipeline_todos - TODO string in output.md: Write this documentation describing your workflow's output

❔ Tests ignored:

  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • files_unchanged - File ignored due to lint config: .gitattributes
  • files_unchanged - File ignored due to lint config: LICENSE or LICENSE.md or LICENCE or LICENCE.md
  • files_unchanged - File ignored due to lint config: .github/CONTRIBUTING.md
  • files_unchanged - File ignored due to lint config: .github/ISSUE_TEMPLATE/bug_report.yml
  • files_unchanged - File ignored due to lint config: .github/PULL_REQUEST_TEMPLATE.md
  • files_unchanged - File ignored due to lint config: .github/workflows/linting_comment.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/email_template.txt
  • files_unchanged - File ignored due to lint config: assets/sendmail_template.txt
  • files_unchanged - File ignored due to lint config: lib/NfcoreSchema.groovy
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore or pyproject.toml
  • multiqc_config - 'assets/multiqc_config.yml' not found

✅ Tests passed:

Run details

  • nf-core/tools version 2.7.2
  • Run at 2023-04-19 09:24:30

Copy link
Member

@muffato muffato left a comment

Choose a reason for hiding this comment

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

How can I test this ? All test profiles fail because of missing data

DLBPointon
DLBPointon previously approved these changes Apr 4, 2023
@weaglesBio
Copy link
Contributor Author

  • Updated findHalfcoverage.py with parameter changes.
  • Replaced SORT_INTERSECT with BEDTOOLS_SORT
  • Replaced GET_FILES_FROM_DIRECTORY with GrabFiles process
  • Added process single to GRAPHOVERALLCOVERAGE and FINDHALFCOVERAGE
  • Removed LONGREADCOVERAGE prefix from local modules

We will be reviewing test data for automated testing shortly. Currently tested locally with both S and L size parameters.

@weaglesBio weaglesBio requested a review from muffato April 6, 2023 14:29
@muffato
Copy link
Member

muffato commented Apr 6, 2023

We will be reviewing test data for automated testing shortly. Currently tested locally with both S and L size parameters.

What's the command-line for running the tests ?

Copy link
Member

@muffato muffato left a comment

Choose a reason for hiding this comment

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

Why did you replace sort with bedtools sort ? The authors themselves say it's not very useful, cf the disclaimer at the bottom of https://bedtools.readthedocs.io/en/latest/content/tools/sort.html

action="store", type="string", dest="depth",
help="depthgraph file, bp count at each depth")
parser.add_option("-w", "--wiggle",
action="store", type="string", dest="wig", default=5,
Copy link
Member

@muffato muffato Apr 7, 2023

Choose a reason for hiding this comment

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

So that the value is kept as a float, not a string

Suggested change
action="store", type="string", dest="wig", default=5,
action="store", type="float", dest="wig", default=5,

action="store", type="string", dest="wig", default=5,
help="wiggle room to add to depth cutoff ie 30X + wiggleroom. Default is 5X")
parser.add_option("--cut",
action="store", type="string", dest="covcut", default=60,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
action="store", type="string", dest="covcut", default=60,
action="store", type="float", dest="covcut", default=60,

Comment on lines 124 to 130

if options.wig == None:
options.wig = 5

if options.covcut == None:
options.covcut = 60

Copy link
Member

@muffato muffato Apr 7, 2023

Choose a reason for hiding this comment

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

Not needed anymore

Suggested change
if options.wig == None:
options.wig = 5
if options.covcut == None:
options.covcut = 60

action="store", type="string", dest="covcut", default=60,
help="%Number for coverage cutoff to include in results. ie 50% of scaffold needs to be under diploid peak etc. Default is 60%")
parser.add_option("-t", "--totalsize",
action="store", type="string", dest="totsize", default=250000,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
action="store", type="string", dest="totsize", default=250000,
action="store", type="int", dest="totsize", default=250000,

@DLBPointon DLBPointon dismissed their stale review April 11, 2023 11:12

muffato knows better

@weaglesBio
Copy link
Contributor Author

Updated findHalfCoverage.py with corrections.
Replaced BEDTOOLS_SORT with GNU_SORT

@weaglesBio weaglesBio requested a review from muffato April 19, 2023 09:25
@weaglesBio weaglesBio merged commit 2828fe8 into dev Apr 19, 2023
@weaglesBio weaglesBio deleted the longread_coverage branch April 19, 2023 10:31
DLBPointon pushed a commit that referenced this pull request Sep 7, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add LONGREAD_COVERAGE subworkflow.
3 participants