-
Notifications
You must be signed in to change notification settings - Fork 5
Simplify the logic in slope plots, add order option & allow download in ZIP #641
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
4f7a2e8
2f76e49
c0ef020
cabdc38
76338b8
daec410
08ef8d6
862ad2f
99c195d
7e4ab70
c8b08e0
3676fe2
02b5f3e
85ea167
16f55db
d83e95c
e0dcd12
bed581c
c67b0a3
84cb307
0bee541
882d4b9
f8fd6ad
847032d
921a7a1
1444a06
85960fc
d26ad10
88a8991
1c1ad55
0e9a1cd
78ec71d
80d66b7
dddc976
f4317e2
1f425f7
569402e
e7af2a7
b2b5a64
f3f2269
7b1e69a
5fb2953
dc227e8
c39ac39
f9ba366
efa0065
94b373d
ab62e7c
1d9ad36
a339f98
4209301
95136da
8f63952
b97b2ac
3135116
20999ae
6c8e920
6ab37f2
38f5dbf
2066d87
7989d78
6881e54
ae64689
df8a87d
e0f350a
5628c01
ab9f9e5
57f577a
32210f1
7bd5d1c
13940d8
bf46eae
2b5f9eb
e805295
cb5e042
fee08b2
0e6f9e8
c1f3812
e00c4e7
50ecaaf
95b3107
a6c4bf9
b11af65
92dbb96
4ecf25f
e144f93
20e100d
b0ee03a
0d840d3
03efeb3
fe17c25
8367d90
e54a3b2
a4f826b
85e31ed
8b39738
1ef1580
039d213
d25281c
82f38d7
be11957
feda4c0
62788c9
f5b4f71
ea6159a
5ee4696
de68700
47a59de
f80637f
edcd5a7
7b544da
3ff0937
b5e4082
1facc21
02fe1e5
9bb2d88
4bb5b65
082c140
2993778
df40ab1
6d26ed2
6f0ea86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,7 +117,7 @@ PKNCA_create_data_object <- function(adnca_data, nca_exclude_reason_columns = NU | |
| # Set default settings | ||
| df_conc$is.excluded.hl <- FALSE | ||
| df_conc$is.included.hl <- FALSE | ||
| df_conc$REASON <- NA | ||
| df_conc$REASON <- "" | ||
| df_conc$exclude_half.life <- FALSE | ||
|
|
||
| # Create PKNCA conc object | ||
|
|
@@ -197,6 +197,8 @@ PKNCA_create_data_object <- function(adnca_data, nca_exclude_reason_columns = NU | |
| #' | ||
| #' Step 5: Impute start values if requested | ||
| #' | ||
| #' Step 6: Indicate points excluded / selected manually for half-life | ||
| #' | ||
| #' Note*: The function assumes that the `adnca_data` object has been | ||
| #' created using the `PKNCA_create_data_object()` function. | ||
| #' | ||
|
|
@@ -205,6 +207,9 @@ PKNCA_create_data_object <- function(adnca_data, nca_exclude_reason_columns = NU | |
| #' @param selected_analytes User selected analytes | ||
| #' @param selected_profile User selected dose numbers/profiles | ||
| #' @param selected_pcspec User selected specimen | ||
| #' @param hl_adj_rules A data frame containing half-life adjustment rules. It must | ||
| #' contain group columns and rule specification columns; | ||
| #' TYPE: (Inclusion, Exclusion), RANGE: (start-end). | ||
| #' @param should_impute_c0 Logical indicating whether to impute start concentration values | ||
| #' @param exclusion_list List of exclusion reasons and row indices to apply to the | ||
| #' concentration data. Each item in the list should have: | ||
|
|
@@ -226,6 +231,7 @@ PKNCA_update_data_object <- function( # nolint: object_name_linter | |
| selected_profile, | ||
| selected_pcspec, | ||
| should_impute_c0 = TRUE, | ||
| hl_adj_rules = NULL, | ||
| exclusion_list = NULL | ||
| ) { | ||
|
|
||
|
|
@@ -269,6 +275,10 @@ PKNCA_update_data_object <- function( # nolint: object_name_linter | |
| PCSPEC %in% selected_pcspec | ||
| ) | ||
|
|
||
| # Update concentration data to indicate points excluded / selected manually for half-life | ||
| if (!is.null(hl_adj_rules)) { | ||
| data <- update_pknca_with_rules(data, hl_adj_rules) | ||
| } | ||
| data | ||
| } | ||
|
|
||
|
|
@@ -650,6 +660,53 @@ PKNCA_hl_rules_exclusion <- function(res, rules) { # nolint | |
| res | ||
| } | ||
|
|
||
|
|
||
| #' Checks Before Running NCA | ||
| #' | ||
| #' This function checks that: | ||
| #' 1) exclusions_have_reasons: all manually excluded half-life points in the concentration data | ||
| #' have a non-empty reason provided. If any exclusions are missing a reason, it stops with an error | ||
| #' and prints the affected rows (group columns and time column). | ||
| #' | ||
| #' @param processed_pknca_data A processed PKNCA data object. | ||
| #' @param exclusions_have_reasons Logical; Check that all exclusions have a reason (default: TRUE). | ||
| #' | ||
| #' @return The processed_pknca_data object (input), if checks are successful. | ||
| #' | ||
| #' @details | ||
| #' - If any excluded half-life points are missing a reason, an error is thrown. | ||
| #' - If no exclusions or all have reasons, the function returns the input object. | ||
| #' - Used to enforce good practice/documentation before NCA calculation. | ||
| #' | ||
| #' @examples | ||
| #' # Suppose processed_pknca_data is a valid PKNCA data object | ||
| #' # check_valid_pknca_data(processed_pknca_data) | ||
| check_valid_pknca_data <- function(processed_pknca_data, exclusions_have_reasons = TRUE) { | ||
| if (exclusions_have_reasons) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: So setting one of the arguments of the function to exclusions_have_reasons <- # some logic to check
if (exclusions_have_reasons) {
processed_pknca_data <- checks_before_running_nca(processed_pknca_data)
}or if we want to make this a function restriction, then the function itself should check it, not rely on the calling logic to tell it the data meets some requirements or not. checks_before_running_nca <- function(processed_pknca_data) {
exclusions_have_reasons <- # ... some logic to check
if (!exclusions_have_reasons) {
warning("Exclusions do not have reasons, could not process the data")
return(processed_pknca_data)
}
# ... rest of the function
}
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you might be right but I don't follow this point. The arg allows the user to decide if they want to make that check or not. It is not for them to escape the check because they don't need it, but more in case they just simply don't want to comply with it For now we only have this check, but is likely that we keep increasing it with more |
||
| excl_hl_col <- processed_pknca_data$conc$columns$exclude_half.life | ||
|
|
||
| if (!is.null(excl_hl_col)) { | ||
| data_conc <- processed_pknca_data$conc$data | ||
| conc_groups <- group_vars(processed_pknca_data$conc) | ||
| time_col <- processed_pknca_data$conc$columns$time | ||
|
|
||
| missing_reasons <- data_conc[[excl_hl_col]] & nchar(data_conc[["REASON"]]) == 0 | ||
| missing_reasons_rows <- data_conc[missing_reasons, ] %>% | ||
| select(any_of(c(conc_groups, time_col))) | ||
|
|
||
| if (nrow(missing_reasons_rows) > 0) { | ||
| stop( | ||
| "No reason provided for the following half-life exclusions:\n", | ||
| "\n", | ||
| paste(capture.output(print(missing_reasons_rows)), collapse = "\n"), | ||
| "\n", | ||
| "Please go to `Slope Selection` table and include it" | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| processed_pknca_data | ||
| } | ||
| #' Add Exclusion Reasons to PKNCAdata Object | ||
| #' | ||
| #' This function adds exclusion reasons to the `exclude` column of the concentration object | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed default REASON from NA to empty string. This is a behavior change that may affect downstream code expecting NA values. Verify that all code checking for missing reasons handles empty strings correctly (e.g., line 739 uses nchar check, which works correctly with empty strings).