Skip to content

Fix tm_outliers joins for compound keys - #1012

Closed
LeonidasZhak wants to merge 1 commit into
insightsengineering:mainfrom
LeonidasZhak:1011_fix_tm_outliers_compound_keys
Closed

Fix tm_outliers joins for compound keys#1012
LeonidasZhak wants to merge 1 commit into
insightsengineering:mainfrom
LeonidasZhak:1011_fix_tm_outliers_compound_keys

Conversation

@LeonidasZhak

Copy link
Copy Markdown

Summary

Preserve complete primary keys in the picks() implementation of tm_outliers() so child datasets with compound keys can populate the outlier table.

Thanks to maintainers

Thanks for maintaining the module and for taking the time to review this focused fix.

Issue or motivation

Fixes #1011. With ADLB selected and no categorical variable, the merged analysis kept only STUDYID and USUBJID, while the outlier table joined back on STUDYID, USUBJID, PARAMCD, and AVISIT.

Root cause

teal.picks::merge_srv() retains the child dataset's foreign key unless other variables are selected. srv_outliers.picks() later needs the complete primary key to retrieve additional columns, so the generated join referenced columns absent from ANL.

Change

Add an internal selector for the selected outlier dataset's primary key before merging. The regression test uses rADSL and rADLB, leaves the categorical selection empty, and checks both complete-key retention and stable row counts.

Tests

  • TESTING_DEPTH=4 NOT_CRAN=true R CMD check --as-cran --no-manual: no errors or warnings; one expected development-version NOTE.
  • TESTING_DEPTH=5 browser tests: tm_outliers passed 61 assertions. The full run's two tm_file_viewer failures were reproduced unchanged on a clean origin/main worktree.
  • Focused and related tm_outliers tests passed (84 assertions).
  • lintr passed on the changed R files; styler reported no changes.

Scope

This only changes the picks() merge inputs and adds regression coverage. It does not change the legacy data_extract_spec path, public arguments, plots, or teal.picks merge behavior.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor


🎉 Thank you for your contribution! Before this PR can be accepted, we require that you read and agree to our Contributor License Agreement.
You can digitally sign the CLA by posting a comment on this Pull Request in the format shown below. This agreement will apply to this PR as well as all future contributions on this repository.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@LeonidasZhak

Copy link
Copy Markdown
Author

Summary

Closing this PR because the fix belongs in the teal.picks merge layer.

Thanks to maintainers

Thanks for correcting the scope and for opening the upstream tracking issues.

Issue or motivation

tm_outliers() expects the merged analysis data to retain the full ADLB primary key before joining additional columns.

Root cause

With teal.picks 0.3.0, selecting ADLB and AVAL produces ANL with STUDYID, USUBJID, and AVAL; PARAMCD and AVISIT are dropped. #119 tracks retaining primary keys, and #120 tracks valid join keys for merged output.

Change

I am withdrawing the module-level selector workaround and closing this PR.

Tests

The focused tm_outliers_picks test passed (2 expectations). A full vignette build and R CMD check --as-cran --no-manual completed with no errors or warnings and one development-version NOTE.

Scope

No branch rewrite or code push was made, and I did not open a duplicate upstream PR.

@github-actions github-actions Bot locked and limited conversation to collaborators Aug 11, 2026
@LeonidasZhak LeonidasZhak reopened this Aug 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: tm_outliers() (picks) errors on a child dataset with a compound primary key: "Join columns in x must be present in the data"

1 participant