Optionally reduce complexity of codebase - #554
Merged
Merged
Conversation
`ruff check --select C901 --duplicate` at max-complexity=10 flagged seven
functions. Each is now under the limit by extracting the phases it was
interleaving; no logic moved between phases and no control flow changed.
score_cell_type_alignment 26 -> 7
load_factors 15 -> <5
plot_cell_type_alignment 14 -> <5
bicv 12 -> <5
cell_type_alignment 12 -> <5
export_factors 11 -> <5
plot_condition_factors 11 -> <5
The largest single win is a genuine duplicate rather than a split:
`cell_type_alignment` and `score_cell_type_alignment` each carried their
own copy of the permutation-null and Mann-Whitney p-value computation,
identical but for the variable names, which is why pylint's
duplicate-code check never paired them. Both now call
`_component_p_values`.
The `n_permutations` dispatch keeps its exact original shape -- a
negative value still leaves the p-values at 1.0 rather than falling
through to the asymptotic test, which a plain `if > 0 / else` would have
changed.
Verified with a golden harness of 69 keys covering all seven functions:
every returned frame, series and array, the rendered PNG hash of eleven
plot configurations, and the type and message of ten error paths. The
harness is bit-identical before and after, and was confirmed
deterministic across repeat runs first. 90 passed / 1 skipped, ruff
clean, `ty` clean over `scrise/`, and no new duplicate-code pairs.
Note these extractions add lines rather than remove them, since each
helper gains a docstring: the five touched files grow by 159 lines net.
Cutting file length means splitting modules, which is a separate
structural decision.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016VJRB1RMUAb1id11XZ6KWS
Extracting helpers for C901 added lines rather than removing them, so the
two files over 650 lines are now split. Both seams already existed; no
function moved between concerns and no body changed.
factorization.py 686 -> 417 + factor_io.py 316
annotation_alignment 695 -> 439 + alignment_stats.py 291
`factor_io` takes everything that moves a decomposition to and from disk
-- OPQ quantization of the projections, the packed barcode matrix, and
the raw-data reattachment -- leaving `factorization` concerned with
computing a decomposition rather than storing one.
`alignment_stats` takes the pure numeric routines: AUROC enrichment,
tau, the two effect sizes, and both p-value nulls. None of them touch
AnnData, so they are now testable and reusable on plain arrays, and
`annotation_alignment` keeps the AnnData plumbing and result assembly.
Every name stays importable from the path it was already on:
`scrise.factorization.export_factors` and
`scrise.annotation_alignment.compute_tau` both still resolve, and to the
same objects as via the new modules. `scrise.__all__` is unchanged and
the `analysis/` figure scripts import unchanged.
Verified with the same 69-key golden harness used for the complexity
work -- every frame, series and array, eleven plot PNG hashes, and ten
error messages, all bit-identical. 90 passed / 1 skipped, ruff clean,
`ty` clean over `scrise/`, C901 clean, vulture clean, and no new
duplicate-code pairs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016VJRB1RMUAb1id11XZ6KWS
The split left `factorization` re-exporting `export_factors`/`load_factors`
purely so old import paths kept resolving, which meant two places to look
for one function. Every call site now names the module that defines what
it wants, and the shim is gone rather than papered over.
scrise/__init__.py factor_io, alignment_stats
scrise/tests/test_contracts.py factor_io
scrise/tests/test_annotation_alignment.py alignment_stats
docs/api.md both, under their own headings
`factorization.__all__` and `annotation_alignment.__all__` now list only
what those modules define. `annotation_alignment` still imports the
statistics helpers, because `cell_type_alignment` and
`score_cell_type_alignment` call them -- that is a real dependency, not a
re-export, and they are no longer advertised as part of that module's
surface.
`scrise/__init__.py` keeps aggregating the whole public API, so
`from scrise import export_factors` and `from scrise import compute_tau`
are unchanged for callers; they now resolve to the defining module, and
`scrise.export_factors is factor_io.export_factors`.
Also carries the docstring trims made directly to `factor_io` and
`alignment_stats`.
Verified: the 69-key golden harness is still bit-identical, 90 passed /
1 skipped, ruff and C901 clean, `ty` clean over `scrise/`, every
`analysis/` module imports, and `mkdocs build --strict` resolves both new
`:::` module references.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016VJRB1RMUAb1id11XZ6KWS
aarmey
approved these changes
Sep 12, 2026
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
2 participants
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.
I am not sure if this is totally necessary but technically this is a much cleaner way of representing the repository as a public API reference. Up to you though if you think it is a value-add or not.
The idea was to factor out conflicting purposes in a single script (factorization spins out load and save factors into factorization_io; alignment statistics are spun out of actual cell-type alignment) and to keep C901 functions under 10.