methylseq icon indicating copy to clipboard operation
methylseq copied to clipboard
verified publisher icon nf-core

Add BISCUIT

Open ekushele opened this issue 4 years ago • 3 comments

nf-core/methylseq pull request

This is a continuation-improved PR of this PR: https://github.com/nf-core/methylseq/pull/147

PR checklist

  • [x] This comment contains a description of changes (with reason)
  • [x] Ensure the test suite passes (nextflow run . -profile test,docker).
  • [x] Make sure your code lints (nf-core lint .).
  • [x] Documentation in docs is updated
  • [x] CHANGELOG.md is updated
  • [x] README.md is updated

Learn more about contributing: CONTRIBUTING.md

ekushele avatar Feb 15 '21 10:02 ekushele

Hi @ekushele,

I just had a quick look over your PR here and fixed the merge conflicts that had arisen / did a tiny bit of whitespace tidying.

So to recap, where are we this now?

  • Need https://github.com/bioconda/bioconda-recipes/pull/26866 to be merged
  • Need to resolve how the large scripts in bin are to be handled
  • Finish review + testing

Is that right? I'm trying to work out whether we should try to get this done before the DSL2 conversion of the pipeline (https://github.com/nf-core/methylseq/pull/199). In some ways the DSL2 conversion could actually make things easier - separate containers means less trouble with conflicting conda environment packages. But it will require a substantial rewrite of this code, notably to use centralised DSL2 module wrappers in nf-core/modules. This will need to be done for the DSL2 conversion anyway though, it's just a question of whether we try to get it merged and released prior to this.

Given that the bioconda issue would be solved by DSL2, I'm inclined to do that first. What do you think? Would that be ok?

Phil

ewels avatar Mar 30 '21 20:03 ewels

nf-core lint overall result: Passed :white_check_mark: :warning:

Posted for pipeline commit 26de2f1

+| ✅ 171 tests passed       |+
#| ❔   1 tests were ignored |#
!| ❗   6 tests had warnings |!

:heavy_exclamation_mark: Test warnings:

  • conda_env_yaml - Conda dep outdated: conda-forge::python=3.8.8, 3.9.2 available
  • conda_env_yaml - Conda dep outdated: bioconda::samtools=1.11, 1.12 available
  • conda_env_yaml - Conda dep outdated: bioconda::preseq=2.0.3, 3.1.2 available
  • conda_env_yaml - Conda dep outdated: bioconda::multiqc=1.10, 1.10.1 available
  • conda_env_yaml - Conda dep outdated: bioconda::bcftools=1.9, 1.12 available
  • conda_env_yaml - Conda dep outdated: conda-forge::parallel=20201122, 20210222 available

:grey_question: Tests ignored:

  • files_unchanged - File ignored due to lint config: lib/NfcoreSchema.groovy

:white_check_mark: Tests passed:

Run details

  • nf-core/tools version 1.13.3
  • Run at 2021-04-04 08:06:28

github-actions[bot] avatar Apr 04 '21 07:04 github-actions[bot]

Replying to this: https://github.com/nf-core/methylseq/pull/186#issuecomment-810567904 @ewels Yes, I think it would be OK

Edit: @ewels Should I do anything in order to help with the DSL2 conversion or other people work on it?

ekushele avatar Apr 04 '21 08:04 ekushele

closing this in favor of #295

sateeshperi avatar May 27 '23 19:05 sateeshperi