Fix focal fill error when personID column uses non-default name#155
Merged
smasongarrison merged 3 commits intomainfrom Apr 7, 2026
Merged
Fix focal fill error when personID column uses non-default name#155smasongarrison merged 3 commits intomainfrom
smasongarrison merged 3 commits intomainfrom
Conversation
- Fix join_by sides in addFocalFillColumn: use dynamic personID on left (ds_ped column) and literal 'personID' on right (fill_df column) - Fix type coercion in createFillColumn: use ped[[personID]] instead of hardcoded ped$personID - Fix error message to show actual column name - Add test for non-standard personID column name with focal fill Agent-Logs-Url: https://github.com/R-Computing-Lab/ggpedigree/sessions/e2c6c87a-e37f-42e8-b68e-bf84f1f6b430 Co-authored-by: smasongarrison <6001608+smasongarrison@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix non-standard personID errors for focal fill
Fix focal fill error when personID column uses non-default name
Apr 7, 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
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.
When
personIDis set to anything other than"personID"(e.g.,personID = "ID"), enabling focal fill throws an error because the join and type-coercion logic hardcoded the literal column name"personID".Changes
addFocalFillColumn()— wrongjoin_bydirectionjoin_by(personID == !!rlang::sym(personID))looked for a literal"personID"column inds_ped(left side). Swapped tojoin_by(!!rlang::sym(personID) == personID)so the left side uses the caller-supplied column name and the right side matches the always-"personID"-named column infill_df.createFillColumn()— hardcodedped$personIDin type coercionis.numeric(ped$personID)returnsNULL(falsy) when the column isn't literally named"personID", silently skipping numeric coercion and causing a type mismatch on join. Changed toped[[personID]].Error message updated to reflect the actual column name rather than the hardcoded string
"ped$personID".New test covers the non-standard column name path with focal fill enabled.