fix(phylo): exclude skipped positions from the identity distance maximum - #5333

Summary

DistanceCalculator with the default identity scoring model returned inflated distances whenever skip_letters was used, because skipped positions (typically gap characters) were removed from the numerator but still counted in the denominator.

Root cause

In Bio/Phylo/TreeConstruction.py, the scoring_matrix is None branch built the score like this:

score = sum(
    l1 == l2
    for l1, l2 in zip(seq1, seq2)
    if l1 not in self.skip_letters and l2 not in self.skip_letters
)
max_score = len(seq1)

The score sum filters out any position where either residue is in skip_letters, but max_score was set to the full alignment length. Since the distance is 1 - score / max_score, every skipped column acts as a permanent penalty: two identical sequences that differ only in gap columns scored 1 - 4/6 = 0.333 instead of 0.0.

The scoring-matrix branch below it already does the right thing (it increments max_score1/max_score2 inside the per-column loop, so skipped columns are excluded from both), which is why the identity branch was the outlier. This matters for real workflows: DistanceCalculator("identity", skip_letters=("-", "*")) combined with DistanceTreeConstructor produces a wrong topology for gap-rich alignments.

Changes

• Bio/Phylo/TreeConstruction.py - count max_score only over positions where neither residue is in skip_letters, so numerator and denominator are built from exactly the same filtered positions. Replaced the sum(...) expression with an explicit loop, because that is what makes the two stay in sync by construction.
• Tests/test_TreeConstruction.py - three regression tests:
  • test_identity_skip_letters_msa: identical sequences with two skipped columns must give distance 0.0 (this is the case that failed before);
  • test_identity_skip_letters: a single substitution over the four scored columns gives 1 - 3/4;
  • test_identity_skip_letters_ignores_gap_count: adding skipped gap columns must not change the distance.
• CONTRIB.rst - added myself to the contributor listing, as required by Biopython's contribution guidelines.

Testing

I could not execute the test suite in my environment: python setup.py build_ext --inplace fails with Microsoft Visual C++ 14.0 or greater is required, so the compiled Bio.Align._aligncore extension is unavailable and Tests/test_TreeConstruction.py cannot even be imported (ImportError: cannot import name '_aligncore'). This is a local toolchain limitation, not a property of the change.

I therefore replayed the old and new scoring blocks verbatim, in isolation, against the same alignments the new tests use:

All assertions passed, and the bug reproduces under the pre-fix logic (1 - 4/6 = 0.333333 instead of 0.0), which is the behaviour the new test_identity_skip_letters_msa locks down. The third row confirms the change is a no-op when nothing is skipped, so existing distance values are unaffected.

The new tests express these same expectations through DistanceCalculator.get_distance on real Align/MSAlignment objects; CI, which builds the C extensions, is what will run them.

Fixes #5332