Skip to content

append_join_keys for teal_data object #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
m7pr wants to merge 8 commits into
base: main
Choose a base branch
from
Open

append_join_keys for teal_data object #373

m7pr wants to merge 8 commits into from

Conversation

@m7pr
Copy link
Contributor

@m7pr m7pr commented Jun 4, 2025 •
edited
Loading 

data <- teal_data()
keys1 <- join_keys(
   join_key("ADSL", "ADSL", "USUBJID"),
   join_key("ADAE", "ADAE", "USUBJID"),
   join_key("ADSL", "ADAE", c(USUBJID = "USUBJID"))
 )
data <- append_join_keys(data, keys1)
join_keys(data)
A join_keys object containing foreign keys between 2 datasets:
ADSL: [USUBJID]
  <-- ADAE: [USUBJID]
ADAE: [USUBJID]
  --> ADSL: [USUBJID]

gogonzo and others added 6 commits May 27, 2025 11:34
@gogonzo
WIP
94ca7ff
@gogonzo
roxy
246bd30
@gogonzo
fix split_code error
427586b
@averissimo
cleanup: remove print debug
bdcba9c
@m7pr
change message in show method for teal_data (#372)
98bd4ca 
I've suggested "code verified/unverified" because:
1. It's concise and clear
2. It focuses on what's actually being verified (the code that created
the data)
3. It removes the redundant "teal_data object" since this is already
known from the context
@m7pr
append_join_keys to teal_data object
243acf4
@m7pr m7pr requested review from averissimo and gogonzo June 4, 2025 12:21
@m7pr m7pr added the core label Jun 4, 2025
@averissimo averissimo self-assigned this Jun 4, 2025
averissimo
averissimo requested changes Jun 4, 2025
View reviewed changes
Copy link
Contributor

@averissimo averissimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One typo on the documentation and 2 minor comments

R/join_keys-append.R
#' join_keys(data)data
#'
#' @export
append_join_keys <- function(x, values, after = length(join_keys(x))) {
Copy link
Contributor

@averissimo averissimo Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the after argument?

IIRC it has no practical effect (all join keys list elements are named and referenced as such).

Copy link
Contributor Author

@m7pr m7pr Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right : D

Copy link
Contributor Author

@m7pr m7pr Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@averissimo so maybe we can change the name to add_join_keys

Copy link
Contributor

@averissimo averissimo Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go for it!

m7pr and others added 2 commits June 6, 2025 09:43
@m7pr @averissimo
Update R/join_keys-append.R
6d198da 
Co-authored-by: André Veríssimo <[email protected]>
Signed-off-by: Marcin <[email protected]>
@m7pr @averissimo
Update R/join_keys-append.R
3c224c6 
Co-authored-by: André Veríssimo <[email protected]>
Signed-off-by: Marcin <[email protected]>
Base automatically changed from teal_reportable to main June 12, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@averissimo averissimo averissimo requested changes

@gogonzo gogonzo Awaiting requested review from gogonzo

Requested changes must be addressed to merge this pull request.

Assignees

@averissimo averissimo

Labels

core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@m7pr @averissimo @gogonzo