Skip to content

Clean up spine math utilities#129

Open
francois-drielsma wants to merge 1 commit into
mainfrom
codex/spine-math-cleanup
Open

Clean up spine math utilities#129
francois-drielsma wants to merge 1 commit into
mainfrom
codex/spine-math-cleanup

Conversation

@francois-drielsma
Copy link
Copy Markdown
Member

Summary

This PR cleans up spine.math, expands focused test coverage, and fixes a few functional issues found during review.

Functional Changes

  • Fixes closest_pair(..., iterative=True): the old loop found a closest point in the opposite set but did not switch sides for the next iteration, so the iterative path was not truly alternating between sets.
  • Reworks union_find to use a parent-array implementation with path compression. Group membership is preserved, while groups keys now match compact returned labels when return_inverse=True.
  • Adds validation for invalid inputs:
    • mismatched label lengths in contingency_table and adjusted_rand_score
    • non-positive KNN k
    • negative radius / epsilon and non-positive DBSCAN min_samples
    • undersampled PCA inputs
  • Fixes call sites that passed "recursive" positionally into boolean iterative.

Cleanup

  • Replaces Numba-specific annotations in signatures with Python / NumPy annotations.
  • Uses named metric constants instead of repeated np.int64(...) casts.
  • Cleans docstring shape notation and typo-level documentation issues.
  • Adds targeted ignores for valid Numba jitclass decorators that Pylance cannot type correctly.

Coverage

  • Adds focused tests under test/test_math.
  • Updates coverage runs to set NUMBA_DISABLE_JIT=1 so coverage can trace jitted function bodies.
  • Normal tests still run with JIT enabled.

Validation

  • .venv/bin/pytest test/test_math -q -> 66 passed
  • NUMBA_DISABLE_JIT=1 .venv/bin/pytest test/test_math --cov=src/spine/math --cov-report=term-missing -q -> 66 passed, 100% coverage
  • bash check_coverage.sh test/test_math/ spine.math -> 66 passed, 100% coverage in Docker
  • .venv/bin/python -m compileall -q src/spine/math test/test_math
  • pre-commit hooks passed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant