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

Add index format to samtools view #7481

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

fellen31
Copy link
Contributor

@fellen31 fellen31 commented Feb 14, 2025

Adds an index_format that specifies index generation explicitly (as suggested by @SPPearce), while still allowing for ext.args to contain --write-index (which generates the index with the default samtools index format for the filetype).

Would supersede #4326.

Also:

  • fixes stub with unselected.
  • Port bam_split_by_region to nf-test.

PR checklist

Closes #XXX

  • 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 module conventions in the contribution docs
  • If necessary, include test data in your PR.
  • Remove all TODO statements.
  • Emit the versions.yml file.
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • nf-core modules test <MODULE> --profile docker
      • nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • nf-core subworkflows test <SUBWORKFLOW> --profile conda

@fellen31 fellen31 force-pushed the samtools-view-index-type branch from 4d38e7a to bb3ec89 Compare February 14, 2025 07:55
@fellen31 fellen31 changed the title Add output index type to samtools view Add index format to samtools view Feb 14, 2025
@fellen31 fellen31 force-pushed the samtools-view-index-type branch from fb0476d to 2c988a4 Compare February 14, 2025 08:17
@fellen31 fellen31 force-pushed the samtools-view-index-type branch 4 times, most recently from 61958a0 to de3c4c1 Compare February 14, 2025 09:31
@fellen31 fellen31 force-pushed the samtools-view-index-type branch from de3c4c1 to 161a736 Compare February 14, 2025 09:44
@fellen31 fellen31 marked this pull request as ready for review February 14, 2025 09:46
@fellen31 fellen31 requested a review from a team as a code owner February 14, 2025 09:46
@fellen31
Copy link
Contributor Author

I believe all remaining lint and pytest-workflow failures are not due to updating this module, but other module updates that I can't adress in this PR.

@SPPearce
Copy link
Contributor

It is trying to lint top-level tools again, rather than the subtool :|
e.g.

Run Linting / nf-core lint modules (graphtyper)

@fellen31
Copy link
Contributor Author

It is trying to lint top-level tools again, rather than the subtool :| e.g.

Run Linting / nf-core lint modules (graphtyper)

Can we exclude the pytests tests/ directory from the CI? Guessing that is why it's trying to lint top-level?

@fellen31 fellen31 force-pushed the samtools-view-index-type branch 3 times, most recently from f085fe4 to 524b9e0 Compare February 14, 2025 10:43
@fellen31 fellen31 force-pushed the samtools-view-index-type branch 2 times, most recently from 558be5d to d13abb1 Compare February 14, 2025 10:56
@fellen31 fellen31 force-pushed the samtools-view-index-type branch from d13abb1 to 16f4dc2 Compare February 14, 2025 11:06
@fellen31
Copy link
Contributor Author

It is trying to lint top-level tools again, rather than the subtool :| e.g.

Run Linting / nf-core lint modules (graphtyper)

Making a separate PR: #7482

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants