Migrate poetry to pixi#85
Merged
Merged
Conversation
check_dependencies() reported "Unicycler not found" for a perfectly working
Unicycler whenever TERM was unset (e.g. inside Snakemake jobs, `mamba run`, or
CI without `export TERM`).
Root cause: Unicycler shells out to `tput`, which prints
`tput: No value for $TERM and no -T specified` to stderr when TERM is unset.
The unicycler check captures with `stderr=sp.STDOUT` (stderr merged into stdout)
and then parsed positionally with `out.split(" ")[1]`, which on the noisy
output returns "No" (from "tput: No value...") instead of the version, raising
and tripping the bare `except` -> "Unicycler not found".
Extract the version parsing into `parse_unicycler_version()` and use a regex
(`Unicycler v(\d+)\.(\d+)\.(\d+)`) that finds the version anywhere in the
output, so stray stderr lines no longer break it. Add unit tests covering clean
output, the TERM-unset/tput-noise regression, and a missing-unicycler case.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "Install plassembler" CI step runs `python -m pip install --upgrade pip` then `pip install -e .`, but the conda env was created without pip (recent conda/miniconda no longer bundles pip by default, and `conda install python=3.12` doesn't add it), so the step failed with "No module named pip". Add pip to the environment so it's always present. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Biopython >=1.85 made SeqIO.parse(handle, "fasta") raise ValueError when the first line is not a FASTA header (leading blank line/comment, or non-FASTA content) instead of yielding no records. That ValueError propagated out of validate_fasta before reaching the `logger.error(...)` branch, so an invalid FASTA crashed with a raw traceback instead of plassembler's clean "not in the FASTA format" -> sys.exit(1) (the ERROR sink added in __init__), and tests/test_plassembler.py::test_non_fasta_input (which expects SystemExit) failed with ValueError. Wrap the parse in try/except ValueError and treat it as an invalid FASTA, so both the raising (>=1.85) and warning (transitional) Biopython behaviours funnel through the same logger.error -> sys.exit path. Valid FASTAs are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace Poetry with Pixi for dependency management, environments, build, and CI, mirroring the dnaapler setup. - pyproject.toml: hatchling build backend, PEP 621 [project] metadata, and [tool.pixi.*] sections carrying the full assembly toolchain (flye, unicycler, minimap2, fastp, chopper, mash, raven, samtools, canu, dnaapler) from bioconda. Drop the pypi-test primary source. - Replace black/isort with ruff; normalise the codebase to ruff and fix the bare except in __init__.py. - Single-source the version: [project] version + importlib.metadata, removing the standalone utils/VERSION file. - justfile, CI, and release workflows now delegate to pixi / setup-pixi; commit pixi.lock (solves linux-64, osx-64, osx-arm64). - Remove superseded requirements.txt and .flake8; trim dev tooling from build/environment.yaml; add .pixi/ to .gitignore.
…tion tests - pyproject.toml: register `slow`/`requires_tool` pytest markers and add a `test-fast` pixi task (`pytest -m 'not slow'`) for an offline, tool-free run. - Mark tool-dependent tests (end-to-end pipeline, external-command wrappers, get_depth/mash, dependency check) as `slow` so the fast subset needs no bioinformatics toolchain. - Mark the DB download test `slow`: it fetches the real ~75MB database over the network and overwrites committed fixtures in place (destructive, offline-unsafe). - Add tests/conftest.py (shared logger->SystemExit sink, tmp_dir fixture) and tests/helpers.py (golden TSV/FASTA comparison utilities with float tolerance and a PLASSEMBLER_WRITE_GOLDEN regen switch). - Add deterministic Tier-A tests locking get_contig_lengths / get_contig_circularity against a committed fixture, plus comprehensive error-path and good-path tests for the input-validation functions. Fast subset: 57 passed, 55 deselected in ~1s.
- input_commands.validate_flye_directory: validate assembly_info.txt instead of checking assembly.fasta twice, so a Flye directory missing its info file no longer wrongly skips long-read assembly. Add a regression test. - depth.collate_depths: remove a dead `enumerate` loop that built an unused list, and add a deterministic test locking the copy-number math. Fast subset: 59 passed, 55 deselected.
Guard the plasmid copy-number division against a missing (no "chromosome" contig), non-numeric, or zero chromosome depth, which previously raised NameError/TypeError or produced inf copy numbers. Output for the normal (numeric chromosome depth) case is unchanged. Add regression tests for the no-chromosome and zero-chromosome-depth cases.
- sam_to_fastq.extract_long_fastqs_slow_keep_fastqs: use sets for read-name membership (was O(n^2) list lookups per alignment) and open all output/SAM handles via context managers so they close even on error. - sam_to_fastq.extract_long_fastqs_fast: pass check=True so a failing samtools raises instead of silently producing an empty plasmid FASTQ. - qc.chopper: wrap the leaked log/output handles in `with`, and rename the Popen locals to stop shadowing the `gzip` module and `chopper` function. Validated with real tools: extract_long_fastqs (fast + slow), chopper/fastp wrappers, and the `long --keep_fastqs` end-to-end path all pass.
The earlier collate_depths hardening set the copy-number column to the string "NA" when chromosome depth was missing or zero. In --no_chromosome mode a fake chromosome of A's (zero depth) is created, so the column became "NA" and combine_depth_mash_tsvs's `.astype(float)` raised `ValueError: could not convert string to float: 'NA'` -- the two CI failures (test_plassembler_run_no_chromosome / _long_no_chromosome). Keep the column float-typed: dividing by a zero chromosome depth yields inf (which the depth filter treats as "keep", matching prior behaviour), and NaN is emitted only when the chromosome depth is genuinely unusable. Regression tests updated, including one that reproduces the .astype(float) failure. Validated: both --no_chromosome end-to-end tests pass with the real toolchain.
Stop the suite mutating committed fixtures: - test_db: the download test installs into a TemporaryDirectory rather than removing/overwriting the committed Plassembler_Test_DB fixtures in place. - test_plass_class: the get_depth / get_depth_long / combine_tsv tests (Plass and Assembly) copy their fixture dir into a TemporaryDirectory and work there, so committed intermediates (e.g. assembly_class/depth/combined_short.sam) are no longer regenerated over the tracked copies. Validated with real tools: the 75MB DB download passes and the plass_class tests pass, both leaving the committed fixtures untouched.
Convert the mash / bam / sam_to_fastq / mapping tests to copy their fixture dir (map_dir or mash_dir) into a TemporaryDirectory and run there, so committed fixtures (mash_dir/mash.tsv, map_dir/*.bam, the sam_to_bam outputs, ...) are no longer overwritten by the suite. Validated with real tools: all 10 converted tests pass and leave the committed fixtures untouched.
Run the chopper/fastp and flye/raven/unicycler tests entirely inside a TemporaryDirectory (copying the read-only input reads in) instead of writing QC/assembly output into the shared tests/test_data tree. Drops the manual rmtree/remove_file cleanup and the now-unused remove_directory import. Validated with real tools: all 8 tests pass and leave the working tree clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.