fix: update.PKNCAresults handles ordered-factor groups and stops warning about unchanged groups - #589
Open
billdenney wants to merge 1 commit into
Open
fix: update.PKNCAresults handles ordered-factor groups and stops warning about unchanged groups#589billdenney wants to merge 1 commit into
billdenney wants to merge 1 commit into
Conversation
…ing about unchanged groups Fixes two defects in update.PKNCAresults() (issue 581): - Joins between the old and new data errored with "incompatible types" when group columns were factors whose levels differed (e.g. the ordered-factor Subject in datasets::Theoph after re-leveling), because dplyr/vctrs cannot combine ordered factors with different levels. All joins in the update machinery (changed-group detection, changed-row filtering, and dropping recalculated groups from the old results) now match group columns by value, coercing factor join keys to character only for matching; the returned data keep their original classes and factor levels. - Updating one group's data warned "No concentration data" for every unchanged group because the intervals were left unfiltered while the concentration and dose data were filtered to the changed groups. The intervals are now filtered to the changed groups symmetrically, so unchanged groups are not calculated and produce no warnings. The set of recalculated groups and the merged result are unchanged. Also return early (keeping existing results) when the new data differ only in ways that change no group's data, e.g. group-block reordering; previously this crashed in getGroups() on an empty recalculation. The v01 vignette sentence explaining the per-subject warnings is updated because the warnings no longer occur. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes #581.
Two fixes: (1) all update-machinery joins now match group keys by value (factor keys coerced to character on join copies only), fixing the vctrs incompatible-ordered-factor error while preserving classes, ordered-ness, and levels in returned data; (2) auto-generated per-group intervals are now filtered to the changed groups like conc and dose already were, so unchanged groups are neither recomputed nor warned about. Plus a guard: an update where no group's data changed returns the existing results with the new data attached (message) instead of erroring on an empty recalculation.
Verification: ordered-factor Theoph update is error-free, warning-free, and
expect_identicalto a full recomputation; unchanged-subject rows byte-identical to originals; character-subject regression guard; the new assertions fail on unfixed main exactly as designed. Full suite: 2,715 pass / 0 fail. The v01 vignette sentence explaining the old warnings is updated (its update example now runs warning-free).🤖 Generated with Claude Code