Solve issue #223 - #224
Merged
Merged
Conversation
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.
The original implementation was only an alias:
That worked when every AE parameter had a predefined
summ_row, but issue 223 introduced a custom parameter:This parameter had a label, but no
summ_row.Root Cause
prepare_ae_summary()produced:Five rows in
outdata$n: population, any AE, no AE, drug-related AE, and drug-related death.Only four values in
outdata$name, because the missingsumm_rowfor drug-related was discarded byunlist().When
format_ae_specific()tried to combine the five formatted rows with four row names, R raised:Why The New Logic Works
In
prepare_ae_summary.R, the function first detects the inconsistency:It only repairs names when necessary, so existing valid summary outputs are unchanged.
For each requested parameter, it uses the standard summary text when available and otherwise falls back to the user-provided label:
Therefore:
Finally, the function delegates to
format_ae_specific()for the actual numeric and percentage formatting. The full argument list is retained soformat_ae_summary()preserves its existing API, defaults, documentation, and named-argument behavior.This approach was chosen because the issue specifically requested changing only
format_ae_summary(). Architecturally, the name mismatch originates during preparation, but the wrapper repairs that mismatch without modifyingprepare_ae_summary()or the shared formatter.