Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Blastdbcmd new module #5482

Merged
merged 25 commits into from
Apr 30, 2024
Merged

Blastdbcmd new module #5482

merged 25 commits into from
Apr 30, 2024

Conversation

toniher
Copy link
Contributor

@toniher toniher commented Apr 15, 2024

This addresses issue #5265

@toniher toniher requested a review from a team as a code owner April 15, 2024 16:21
@toniher toniher requested review from CarsonJM and removed request for a team April 15, 2024 16:21
@toniher toniher added the new module Adding a new module label Apr 15, 2024
jfy133
jfy133 reviewed Apr 16, 2024
View reviewed changes
Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

Two main things:

  • You must not have two modules under one header, these should be separate modules
  • You should not remove ${prefix} and rely purely on meta.id, this is the prefix default anyway

modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/tests/main.batch.nf.test Outdated Show resolved Hide resolved
jfy133
jfy133 reviewed Apr 26, 2024
View reviewed changes
Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

This looks much easier/better now! Some remaining comments

modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/main.nf Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/meta.yml Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/tests/main.nf.test Show resolved Hide resolved
@toniher
Copy link
Contributor Author

toniher commented Apr 27, 2024

I think comments addressed, @jfy133 . It also allows now more generic txt outputs.

@jfy133 jfy133 self-requested a review April 28, 2024 07:53
jfy133
jfy133 reviewed Apr 28, 2024
View reviewed changes
Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

Almost there! Just some cleanup /refinement now.

modules/nf-core/blast/blastdbcmd/main.nf Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/main.nf Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/blast/blastdbcmd/tests/nextflow.txt.config Show resolved Hide resolved
tests/config/test_data.config Outdated Show resolved Hide resolved
jfy133
jfy133 approved these changes Apr 30, 2024
View reviewed changes
modules/nf-core/blast/blastdbcmd/meta.yml Outdated Show resolved Hide resolved
@toniher toniher added this pull request to the merge queue Apr 30, 2024
Merged via the queue into nf-core:master with commit b00ebb6 Apr 30, 2024
11 checks passed
@toniher toniher deleted the blastdbcmd branch April 30, 2024 08:08
adamrtalbot pushed a commit to adamrtalbot/modules that referenced this pull request Apr 30, 2024
@toniher @jfy133 @adamrtalbot
Blastdbcmd new module (nf-core#5482)
4441c29 
* starting blastdbcmd

* carry on

* Making it work with simple entry version

* Upgrade to make entry_batch work

* Upgrade to make it work with tests

* adding missing tag

* Removed versions to make it pass the tests in Github

* Make it work with versions and so

* Update modules/nf-core/blast/blastdbcmd/meta.yml

Co-authored-by: James A. Fellows Yates <[email protected]>

* Update modules/nf-core/blast/blastdbcmd/meta.yml

Co-authored-by: James A. Fellows Yates <[email protected]>

* Move module into two and upgrade according to comments

* upgrade with outfmt forced

* Turn back into one module

* Update modules/nf-core/blast/blastdbcmd/main.nf

Co-authored-by: James A. Fellows Yates <[email protected]>

* update tags and stub test

* fix stub

* making it work more widely

* editorcheck error

* addressing comments

* Update modules/nf-core/blast/blastdbcmd/meta.yml

Co-authored-by: James A. Fellows Yates <[email protected]>

---------

Co-authored-by: James A. Fellows Yates <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request May 1, 2024
@adamrtalbot @LouisLeNezet
@k1sauce @mari-ga @jasmezz @Lucpen @rannick @FloWuenne @GallVp @nvnieuwk @wxicu @renovate @asp8200 @edmundmiller @LeonHafner @SPPearce @lrauschning @vlebars @DavideBag @iraiosub @ashotmarg @toniher @jfy133 @CarsonJM
Try alternative strategy for extracting tags (#4928)
ac58a02 
* Try alternative strategy for extracting tags

* Add check to prevent confirm pass if no tags present

* Fix some names for GHA steps

* Add fake change in fastp module

* Add fake change in bam_sort_stats_samtools module

* Add fake changes to pytest module and subworkflow

* Add debug step

* fixup

* Got some variable names wrong...

* Add subworkflows back to nf-tests

* revert confirm-pass if statement

* use JQ to separate modules and subworkflows

* Fix quoting when parsing tags

* coerce modules and subworkflows to lower case

* Add specific linting checks to dependencies

* Single vs double quotes again

* Some more variable fixing

* this might work

* fixup

* yet another fixup

* Correct modules -> subworkflows

* Fake change to pytest module

* Correct dependency

* One more time

* More sorting stuff out

* Update modules/nf-core/gunzip/tests/main.nf.test

* Add fake change to module without nf-test so pytest ONLY is triggered

* fixup incorrect variable

* Switch to using self-hosted Docker profile

* Add dependency checker from Carson Miller

* fixup

* fix(.github/python/find_changed_files.py): detect nf files less greedily and ignore tests directory

The scanning function was picking up nf files outside of the target repos. This change only scans modules, subworkflows and workflows.

* Revert to directories instead of files for module tagging

* Create optional returntype for Python script

* Remove superfluous python cache

* feat: replace custom python with reusable github action for detecting nf-test changes

* Fix variable name in output paths

* Switch to new parents dir syntax

* prod to bump version of action

* Update .github/workflows/test.yml

* Update .github/workflows/test.yml

* Exclude more subworkflows from Conda

* Update .github/workflows/test.yml

* Update .github/workflows/test.yml

* QUILT nf-test and bamlist (#5515)

* Migrate quilt to nf-test and swithc bamlist to auto generation inside the module

* Fix tests

* Fix test

* Fix tests

* Add snap

* Update license

* Add quilt tag

* Specify build hash for conda environment

* Specify build hash for conda environment

* Undo commit

* Update nf-test snapshot

* Fix config name

* Update snapshot

* Update meta.yml

* Add r-base to environment

* Update snapshot generation

* Update meta.yml

* Update snap

* Update snap

* Force sorting

* Update snapshot

* New module: gnu/split (#5428)

* dev

* dev

* dev

* add tests

* fix lint

* fix(gnu/split): pin coreutils in conda to match singularity and docker

* fix: minor formatting fixes, use modules_testdata_base_path in nf-test

---------

Co-authored-by: Kyle <[email protected]>

* Add Demuxem (#5504)

* Fix lint test

* added input

* inputs and outputs demuxem

* updates demuxem, missing snap test

* fixes

* out new parameters testing

* tests passed

* lint test check

* editorconfig fixes

* more fixes editorconfig

* fix editorconfig

* line 24 whitespace

* blank spaces

* fix line 23

* test to fix trailing whitespace

* fix line23

* fix main

* Fix AMRFinderPlus (#5521)

Fix folder renaming

* fix stubs salmon (#5517)

* fix stubs

* fix stub¨

* Update modules/nf-core/salmon/quant/main.nf

---------

Co-authored-by: Lucpen <[email protected]>
Co-authored-by: Annick Renevey <[email protected]>

* Update mcquant: Added nf-test and meta.yml information (#5507)

* Transition to nf-test and meta.yml info fix.

* Added MIT license.

* Removed conda environment.yml for mcquant.

* Added conda exclusion for nf-test.

* Changed to quay.io container.

* Removed quay.io from container

* Added repeatmodeler/builddatabase (#5416)

* Added repeatmodeler/builddatabase

* Fixed version extraction

* Now doing md5 check on stable db files

* bump wisecondorx to v1.2.7 (#5523)

* Add Freemuxlet to modules (#5520)

* add freemuxlet

* add snapshot

* pass container test

* remove trailing whitespace

* add optional output files creation

* remove ldist

* fix test

* add snapshot

---------

Co-authored-by: Xichen Wu <[email protected]>
Co-authored-by: wxicu <[email protected]>

* chore(deps): update pre-commit hook astral-sh/ruff-pre-commit to v0.4.2 (#5527)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Updating sentieon to 202308.02 (#5525)

* Updating sentieon to 202308.02

* Fixing typo

* Fix stale action (#5530)

* ci: Try not setting the repo-token

* ci: Run clean-up on dispatch

* Changed tag from single quotes to double quotes (#5531)

Co-authored-by: Simon Pearce <[email protected]>

* [TYPO] Align commas (#5380)

Align commas

Co-authored-by: Simon Pearce <[email protected]>

* GFFREAD: updated to 0.12.7, added meta and fasta input/output (#5448)

* GFFREAD: updated to 0.12.7, added meta and fasta input/output

* Added a comment explaining args_sorted

* re delete gatk4/bedtointervallist from pytest_modules.yml after it was added again (see #5251) (#5328)

delete gatk4/bedtointervallist from pytest_modules.yml

Co-authored-by: LE BARS Victor <[email protected]>

* Update mosdepth to 0.3.8 (#5538)

* Update mosdepth to 0.3.8

* Updating main.nf.test.snap

* migrating flash to nf-test (#5470)

* migrating flash to nf-test

* removing args2 from main.nf

* Update nextflow.config

* Changes discussed in the outputs

---------

Co-authored-by: Simon Pearce <[email protected]>

* add riboseq transcriptome bam to test config (#5537)

* add ribo-seq transcriptome bam

* remove extra line

* Update nanoplot/main.nf (#5460)

Update main.nf

Updated the "def input_file" section so that it not only takes fastq file with ".fastq.gz" suffix but also ".fq.gz".

* Added repeatmodeler/repeatmodeler (#5536)

* repeatmodeler/repeatmodeler

* Demoted to process_medium

* Update modules/nf-core/repeatmodeler/repeatmodeler/main.nf

Co-authored-by: Simon Pearce <[email protected]>

---------

Co-authored-by: Simon Pearce <[email protected]>

* Blastdbcmd new module (#5482)

* starting blastdbcmd

* carry on

* Making it work with simple entry version

* Upgrade to make entry_batch work

* Upgrade to make it work with tests

* adding missing tag

* Removed versions to make it pass the tests in Github

* Make it work with versions and so

* Update modules/nf-core/blast/blastdbcmd/meta.yml

Co-authored-by: James A. Fellows Yates <[email protected]>

* Update modules/nf-core/blast/blastdbcmd/meta.yml

Co-authored-by: James A. Fellows Yates <[email protected]>

* Move module into two and upgrade according to comments

* upgrade with outfmt forced

* Turn back into one module

* Update modules/nf-core/blast/blastdbcmd/main.nf

Co-authored-by: James A. Fellows Yates <[email protected]>

* update tags and stub test

* fix stub

* making it work more widely

* editorcheck error

* addressing comments

* Update modules/nf-core/blast/blastdbcmd/meta.yml

Co-authored-by: James A. Fellows Yates <[email protected]>

---------

Co-authored-by: James A. Fellows Yates <[email protected]>

* Remove fake changes

Co-authored-by: Carson J Miller <[email protected]>

* Use merge group base ref instead of PR base_ref when merging

* fixup

* Use SHA instead of head ref

* fixup

* work

* origin/

* cmon

* one more time with feeling

* fixup again

* Worth a try

* Increase fetch depth to 2

* Use SHA instead of head ref

---------

Co-authored-by: Louis LE NEZET <[email protected]>
Co-authored-by: Kyle Hazen <[email protected]>
Co-authored-by: Kyle <[email protected]>
Co-authored-by: Mylène Mariana Gonzales André <[email protected]>
Co-authored-by: Jasmin Frangenberg <[email protected]>
Co-authored-by: Lucía Peña-Pérez <[email protected]>
Co-authored-by: Lucpen <[email protected]>
Co-authored-by: Annick Renevey <[email protected]>
Co-authored-by: Florian Wuennemann <[email protected]>
Co-authored-by: Usman Rashid <[email protected]>
Co-authored-by: Nicolas Vannieuwkerke <[email protected]>
Co-authored-by: Xichen Wu <[email protected]>
Co-authored-by: Xichen Wu <[email protected]>
Co-authored-by: wxicu <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Anders Sune Pedersen <[email protected]>
Co-authored-by: Edmund Miller <[email protected]>
Co-authored-by: Leon Hafner <[email protected]>
Co-authored-by: Simon Pearce <[email protected]>
Co-authored-by: Leon Rauschning <[email protected]>
Co-authored-by: vlebars <[email protected]>
Co-authored-by: LE BARS Victor <[email protected]>
Co-authored-by: DavideBag <[email protected]>
Co-authored-by: iraiosub <[email protected]>
Co-authored-by: Ashot Margaryan <[email protected]>
Co-authored-by: Toni Hermoso Pulido <[email protected]>
Co-authored-by: James A. Fellows Yates <[email protected]>
Co-authored-by: Carson J Miller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfy133 jfy133 jfy133 approved these changes

@CarsonJM CarsonJM Awaiting requested review from CarsonJM CarsonJM was automatically assigned from nf-core/nf-test

@JoseEspinosa JoseEspinosa Awaiting requested review from JoseEspinosa

Assignees
No one assigned
Labels
new module Adding a new module
Projects
Hackathon: May 2024
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@toniher @jfy133