Fix population-label handling in boot.ppfst - #81
Open
thierrygosselin wants to merge 6 commits into
Open
thierrygosselin wants to merge 6 commits into
thierrygosselin wants to merge 6 commits into
Conversation
Clarify that confidence-interval row names identify the populations and that single-population input supports numeric, character, and factor identifiers.
Remove the leftover documentation change from master so future branches start without unrelated changes.
Map observed population identifiers to consecutive internal integers before pairwise calculations. Support nonconsecutive numeric labels, character labels, and factors with unused levels while preserving output population labels. Reject missing identifiers and fewer than two observed populations explicitly. The estimator and bootstrap procedure remain unchanged.
Test equivalent intervals for consecutive and nonconsecutive numeric labels, character labels, and factors with unused levels. Verify population-label preservation, factor ordering, and invariance to sample-row order. Check that missing identifiers and fewer than two observed populations produce clear errors.
Document support for numeric, character, and factor population identifiers. Clarify that identifiers must be non-missing, at least two observed populations are required, and unused factor levels are ignored.
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.
Problem
boot.ppfst()uses consecutive integers in its pairwise loop without consistently mapping the input population identifiers to those integers.Consequently, valid data with population labels such as 10 and 20, or A and B, can fail even though the same genotypes work with labels 1 and 2. Unused factor levels can also introduce nonexistent populations.
Changes
Validation
All 24 local regression checks passed without warnings.
Tests cover nonconsecutive numeric identifiers, character labels, factor ordering, unused levels, sample-row permutations, and invalid population identifiers.
The original nonconsecutive-label failure was reproduced and corrected. A separate seeded comparison using gtrunchier confirmed unchanged confidence intervals for ordinary input.
Scope
The Weir–Cockerham estimator and locus-bootstrap procedure are unchanged.
This PR addresses population-label handling only. Separate single-locus and array-input problems are outside its scope.