Skip to content

Conversation

@wwood
Copy link
Owner

@wwood wwood commented Jan 28, 2026

Motivation

  • Record the DIAMOND executable version immediately before running the DIAMOND blastx prefilter so the version is visible in logs for debugging and reproducibility.
  • Add a test assertion to ensure the version log is actually emitted to stderr during the prefilter flow.

Description

  • Call extern.run("diamond --version") and logging.info("DIAMOND version: %s", diamond_version) in singlem/pipe.py immediately before the DIAMOND prefilter blastx invocation.
  • Modify test/test_pipe.py::test_fast_protein_package_prefilter to run the pipe command with subprocess.run(..., capture_output=True) and assert that the stderr contains the "DIAMOND version:" message while preserving the existing stdout OTU table assertion.

Testing

  • Ran the full test suite with pixi run -e dev pytest test to exercise the change and capture CI-style results.
  • Test summary: 224 passed, 1 failed, 20 skipped, 1 warning; the failing test is test/test_pipe.py::Tests::test_genome_input_dereplication which exhibited an assertion mismatch (unrelated to the added logging).
  • The new stderr assertion for the prefilter test executed and the DIAMOND version log was checked during the run.

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20d8aaaf8b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +389 to +390
diamond_version = extern.run("diamond --version").strip()
logging.info("DIAMOND version: %s", diamond_version)

Choose a reason for hiding this comment

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

P2 Badge Guard DIAMOND version check to preserve SRA cleanup

If diamond --version fails (e.g., DIAMOND missing or misconfigured) and input_sra_files was used, the exception now happens before the existing try/except block that logs the DIAMOND failure and calls finish_sra_extraction_processes. That means the kingfisher extraction processes started above can be left running or blocked on their FIFOs with no cleanup, which didn’t occur before this change. This only shows up when SRA input is active and the version check errors, but in that case it can leak subprocesses and leave temp resources behind.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant