Skip to content

Conversation

@ShiyuC
Copy link
Collaborator

@ShiyuC ShiyuC commented May 19, 2025

closes #116

  • Create vignettes for DM and AE domains
  • Update study_ct file

@github-actions
Copy link

github-actions bot commented May 19, 2025

Code Coverage

Package Line Rate Health
sdtm.oak 94%
Summary 94% (1170 / 1241)

Copy link
Collaborator

@rammprasad rammprasad left a comment

Choose a reason for hiding this comment

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

GOod job,. Looks awesome. Minor changes.

)
```

## Create reference dates configuration file {#refdates}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add the column definitions here? It will be easier to the user to understand. Ideally, the user should understand how to prepare this file by looking at this Vignette

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added definitions of each column

Comment on lines 157 to 160
dm <-
# Map AGE using assign_no_ct
assign_no_ct(
raw_dat = dm_raw,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am gettting a CT error while execuring this chunk. Is it working for you?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It works fine for me. Could you double check if the study_ct file you loaded contains 138 observations?

# Map DTHDTC using oak_cal_ref_dates
oak_cal_ref_dates(ds_in = .,
der_var = "DTHDTC",
min_max = "max",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for death date, it has to be min.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

# Derive RFSTDTC using oak_cal_ref_dates
oak_cal_ref_dates(ds_in = .,
der_var = "RFSTDTC",
min_max = "min",
Copy link
Collaborator

Choose a reason for hiding this comment

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

It will be beneficial to explain how to choose min or max for each function call. Explain the derivation of RFSTDTC and explain the parameters, like min or max. Lets just do it for one min and one max examples and then rest doesn't need it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added separate descriptions for RFSTDTC and RFENDTC explaining the selection of min/max. Please review again and see how it looks to you

@github-project-automation github-project-automation bot moved this from Product Backlog to In review in sdtm.oak R package May 20, 2025
ShiyuC and others added 11 commits May 20, 2025 18:49
Merge remote-tracking branch 'origin/main' into 116-docs-aedm
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@ramiromagno ramiromagno linked an issue May 29, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Bug: cal_min_max_date() fails when time_variable has NA values Documentation: Vignettes for creating DM and AE domains

2 participants