High dose hook #182
High dose hook #182
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #182 +/- ##
==========================================
+ Coverage 87.16% 87.50% +0.33%
==========================================
Files 14 14
Lines 1581 1648 +67
==========================================
+ Hits 1378 1442 +64
- Misses 203 206 +3 ☔ View full report in Codecov by Sentry. |
| #' | ||
| is.decreasing <- function(x) { | ||
| stopifnot(is.numeric(x) || is.null(x)) | ||
| if (any(is.na(x))) { |
There was a problem hiding this comment.
if this function is gonna be used only in handle_high_dose_hook then as a user I would prefer to see different error message. I know that this check fits here but I would move it to handle_high_dose_hook and there gave it different, more specific error message such as "NA values detected in the dilutions while handling high dose hook" or something similar
There was a problem hiding this comment.
I guess such a situation should not happen. In general, I think, it would mean that a standard curve sample has a missing dilution value
There was a problem hiding this comment.
But I'd love to see a test in which we process a plate with high-hook
There was a problem hiding this comment.
When it comes to the function is.decreasing the na handling is there because this function is located in the helpers.R and provided as a utility function to be used across the package.
I will add a doc with an explanation about it to the handle_high_dose_hook and create a test specific for phenomenon
There was a problem hiding this comment.
I suggest to approve these changes. In general, if there were a problem with NA dilutions, it would also affect different parts of model creation. There are no specific exceptions thrown that could help track if there is standard curve sample without dilution.
I think, if @nizwant agrees we can approve these changes and optionally add this problem as a new issue, but I don't think that this is necessary.
There was a problem hiding this comment.
I tried to manipulate the plate object so that the standard curve sample would not have the dilution value, and it turns out that then it is read as a test sample instead. I am not sure how to enforce the behaviour that @nizwant wants to avoid other than through manual modification of plate object data.
Implement high dose hook phenomenon handling as described in #179