Fix crashes: sklearn n_neighbors=0, NameError in mcluster, argparse types#6
Open
rec3141 wants to merge 4 commits into
Open
Fix crashes: sklearn n_neighbors=0, NameError in mcluster, argparse types#6rec3141 wants to merge 4 commits into
rec3141 wants to merge 4 commits into
Conversation
kneighbors_graph raises InvalidParameterError when n_neighbors=0 in scikit-learn >= 1.2. This happens when latent.shape[0] is 1 (first clustering pass) or when all contigs are assigned in the first pass leaving 0-1 unclustered contigs for the recluster phase. Guard both kneighbors_graph call sites with an early return when min_k < 1 instead of passing an invalid parameter. Traceback without fix (sklearn 1.8.0): sklearn.utils._param_validation.InvalidParameterError: The 'n_neighbors' parameter of kneighbors_graph must be an int in the range [1, inf). Got 0 instead.
The mcluster() function (multi-sample mode) referenced `samplename` (a local variable inside msample()) instead of the returned `samplenames`. Also fixed `output` → `outdir` to match the function parameter name. Both bugs cause NameError when running LorBin with --multi flag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Both arguments were missing `type=` in add_argument(), causing command-line values to arrive as strings instead of int/float. This causes TypeError on comparisons like `bin_length <= 0`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add missing type= annotations to 8 arguments across the cluster and train subparsers: --batch_size (type=int), --epoch (type=int), --lrate (type=float), and --batchsteps (type=int) in both. Without type=, argparse treats CLI-provided values as strings, causing TypeError on numeric comparisons in train_vae() (e.g. nepochs < 1) and DataLoader(batch_size=...). Also fix help text: correct "default: 64" → "default: 128" to match the actual default, and fix "batchseteps" typo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rec3141
added a commit
to rec3141/danaSeq
that referenced
this pull request
Feb 22, 2026
Idempotent script that applies four open PRs not yet merged upstream: - LorMeBioAI/LorBin#6: Fix sklearn n_neighbors=0 crash, NameError in mcluster, missing argparse type= annotations - LorMeBioAI/LorBin#7: Pass --cuda flag to train_vae in bin subcommand - LottePronk/whokaryote#13: Fix GFF3 parsing for Bakta/PGAP output - oschwengers/bakta#429: Make AMRFinderPlus failure non-fatal (warn + return) Run once after install.sh. Safe to re-run. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
This PR fixes several runtime crashes in LorBin:
Fix
kneighbors_graphcrash with scikit-learn >= 1.2 — Whenlatent.shape[0]is 0 or 1 (e.g., very few contigs),n_neighborsis computed as 0 or negative, which raisesInvalidParameterErrorin scikit-learn >= 1.2. Added early-return guards at bothkneighbors_graphcall sites inbin_cluster().Fix
NameErrorinmcluster()(multi-sample mode) — Two variable name mismatches:samplename→samplenames(to matchmsample()return value) andoutput→outdir(to match the function parameter). Both causeNameErrorat runtime when using--multi.Fix argparse
type=coercion for all numeric arguments — 10 arguments acrossbin_mode,cluster, andtrainsubparsers were missingtype=intortype=float, causingTypeErrorwhen values are provided via CLI (argparse defaults tostr). Also corrects misleading help text (wrong defaults, "batchseteps" typo).Arguments fixed
bin_mode,generate_data,cluster)--bin_lengthtype=intbin_mode,cluster)--akeeptype=floatcluster--batch_sizetype=intcluster--epochtype=intcluster--lratetype=floatcluster--batchstepstype=inttrain--batch_sizetype=inttrain--epochtype=inttrain--lratetype=floattrain--batchstepstype=intRelated issues
Test plan
lorbin binwith a small dataset to confirm normal operationlorbin clusterwith explicit--epoch 100 --batch_size 64to verify type coercionlorbin bin --multiwith multi-sample input to verify mcluster path🤖 Generated with Claude Code