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

correct parameter to use lca for multiple matches in ganon classify #523

Merged
merged 4 commits into from
Sep 11, 2024
Merged

correct parameter to use lca for multiple matches in ganon classify #523

merged 4 commits into from
Sep 11, 2024

Conversation

LilyAnderssonLee
Copy link
Contributor

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/taxprofiler 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>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,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).

@LilyAnderssonLee
Copy link
Contributor Author

Errors from test_full:

The exit status of the task that caused the workflow execution to fail was: 2

Error executing process > 'NFCORE_TAXPROFILER:TAXPROFILER:PROFILING:GANON_CLASSIFY (ganon-db|MOCK_002_Illumina_Hiseq_3000)'

Caused by:
  Essential container in task exited


Command executed:

  dbprefix=$(find -L . -name '*.*ibf' | sed 's/\.h\?ibf$//')
  
  ganon \
      classify \
      --db-prefix ${dbprefix%%.*ibf} \
       --output-all --output-lca --output-unclassified \
      --threads 12 \
      --output-prefix MOCK_002_Illumina_Hiseq_3000_ganon-db.ganon \
      --single-reads MOCK_002_Illumina_Hiseq_3000_1.unmapped.fastq.gz         2>&1 | tee MOCK_002_Illumina_Hiseq_3000_ganon-db.ganon.log
  
  cat <<-END_VERSIONS > versions.yml
  "NFCORE_TAXPROFILER:TAXPROFILER:PROFILING:GANON_CLASSIFY":
      ganon: $(echo $(ganon --version 2>1) | sed 's/.*ganon //g')
  END_VERSIONS

Command exit status:
  2

Command output:
  usage: ganon [-h] [-v]
               {build,build-custom,update,classify,reassign,report,table} ...
  ganon: error: unrecognized arguments: --output-lca

Command error:
  usage: ganon [-h] [-v]
               {build,build-custom,update,classify,reassign,report,table} ...
  ganon: error: unrecognized arguments: --output-lca
  3:27PM INF shutdown filesystem start
  3:27PM INF shutdown filesystem done

Work dir:
  s3://nf-core-awsmegatests/work/taxprofiler/work-8949ad471b0dc855c717a220657d282204e69dc4/02/98c44ab57f8abfd52e82483719450f

Tip: view the complete command output by changing to the process work dir and entering the command `cat .command.out`

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 10, 2024 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 8bf5833

+| ✅ 268 tests passed       |+
!| ❗   1 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in ci.yml: You can customise CI pipeline run tests as required

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-09-11 02:56:50

@jfy133
Copy link
Member

jfy133 commented Sep 11, 2024

Actually we should update this to include to a new parameters to give the user the 3 options... (Skip, em, lca)

@jfy133 jfy133 self-requested a review September 11, 2024 02:50
jfy133
jfy133 approved these changes Sep 11, 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.

Ok I went and looked again (reviewing 4.30am not a good idea) and when checking the ganon docs I found out:

  • -m should now come in via db_params in the database sheet in taxprofiler (we don't need a parameter)
  • because of the change of the -m from a single option (lca) to multiple options, these all go into either .all or .one, so I've updated the pattern accordingly

@jfy133 jfy133 merged commit cc34d41 into bouncy-basenji Sep 11, 2024
24 checks passed
@jfy133 jfy133 deleted the ganon-classy-lca branch September 11, 2024 03:28
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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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