-
Notifications
You must be signed in to change notification settings - Fork 5
Feature diann anomaly #119
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
Changes from all commits
4f6941d
46bfc89
2e0f812
7ec8752
ca4335c
4023a70
c28b200
6a764d0
67a5090
692546b
6f18ab0
004766b
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 |
|---|---|---|
|
|
@@ -8,4 +8,6 @@ inst/doc | |
| *.log | ||
| *.o | ||
| *.so | ||
| *.dll | ||
| *.dll | ||
| .lintr | ||
| .vscode | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -22,6 +22,15 @@ | |||||
| #' @param quantificationColumn Use 'FragmentQuantCorrected'(default) column for quantified intensities for DIANN 1.8.x. | ||||||
| #' Use 'FragmentQuantRaw' for quantified intensities for DIANN 1.9.x. | ||||||
| #' Use 'auto' for quantified intensities for DIANN 2.x where each fragment intensity is a separate column, e.g. Fr0Quantity. | ||||||
| #' @param calculateAnomalyScores Default is FALSE. If TRUE, will run anomaly detection model and calculate anomaly scores for each feature. Used downstream to weigh measurements in differential analysis. | ||||||
| #' @param anomalyModelFeatures character vector of quality metric column names to be used as features in the anomaly detection model. List must not be empty if calculateAnomalyScores=TRUE. | ||||||
| #' @param anomalyModelFeatureTemporal character vector of temporal direction corresponding to columns passed to anomalyModelFeatures. Values must be one of: `mean_decrease`, `mean_increase`, `dispersion_increase`, or NULL (to perform no temporal feature engineering). Default is empty vector. If calculateAnomalyScores=TRUE, vector must have as many values as anomalyModelFeatures (even if all NULL). | ||||||
|
||||||
| #' @param anomalyModelFeatureTemporal character vector of temporal direction corresponding to columns passed to anomalyModelFeatures. Values must be one of: `mean_decrease`, `mean_increase`, `dispersion_increase`, or NULL (to perform no temporal feature engineering). Default is empty vector. If calculateAnomalyScores=TRUE, vector must have as many values as anomalyModelFeatures (even if all NULL). | |
| #' @param anomalyModelFeatureTemporal character vector of temporal direction corresponding to columns passed to anomalyModelFeatures. Values must be one of: `mean_decrease`, `mean_increase`, `dispersion_increase`, or a sentinel indicating no temporal feature engineering (e.g. `none` or `NA_character_`). Default is empty vector. If calculateAnomalyScores=TRUE, vector must have as many values as anomalyModelFeatures (even if all indicate no temporal feature engineering). |
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.
Add upfront validation for anomaly-model arguments.
When calculateAnomalyScores=TRUE, there is no early check that anomalyModelFeatures is non-empty and aligned with anomalyModelFeatureTemporal. This can fail late in Line 168+ with less actionable errors.
Proposed fix
anomalyModelFeatures = .standardizeColnames(anomalyModelFeatures)
+ if (calculateAnomalyScores) {
+ if (length(anomalyModelFeatures) == 0) {
+ stop("anomalyModelFeatures must be non-empty when calculateAnomalyScores=TRUE.")
+ }
+
+ allowed_temporal = c("mean_decrease", "mean_increase", "dispersion_increase", NA_character_)
+ if (length(anomalyModelFeatureTemporal) > 0) {
+ if (length(anomalyModelFeatureTemporal) != length(anomalyModelFeatures)) {
+ stop("anomalyModelFeatureTemporal must have the same length as anomalyModelFeatures.")
+ }
+ if (!all(anomalyModelFeatureTemporal %in% allowed_temporal)) {
+ stop("anomalyModelFeatureTemporal values must be one of mean_decrease, mean_increase, dispersion_increase, or NA/NULL.")
+ }
+ }
+ }Also applies to: 85-86, 167-171
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@R/converters_DIANNtoMSstatsFormat.R` around lines 75 - 78, When
calculateAnomalyScores is TRUE, add upfront validation that anomalyModelFeatures
is non-empty and that length(anomalyModelFeatures) matches
length(anomalyModelFeatureTemporal) (or that anomalyModelFeatureTemporal is
length 1 or same length) before any downstream processing; in practice, inside
the top-level function that accepts args (check the parameter block containing
calculateAnomalyScores, anomalyModelFeatures, anomalyModelFeatureTemporal, e.g.,
the converter function in R/converters_DIANNtoMSstatsFormat.R) add checks that
throw informative errors (stop(...)) if anomalyModelFeatures is NULL/length 0 or
if the two feature vectors are misaligned, and run these validations immediately
after parsing arguments (before any use around lines ~167-171 and ~168) so
failures are early and actionable.
Copilot
AI
Mar 2, 2026
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.
This converter now exposes anomaly-model parameters (calculateAnomalyScores, anomalyModelFeatures, temporal settings, etc.) but doesn’t validate them (unlike SpectronauttoMSstatsFormat, which calls .validateMSstatsConverterParameters). Please add the same validation here to catch cases like calculateAnomalyScores=TRUE with empty/length-mismatched feature vectors early with a clear error.
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.
@copilot open a new pull request to apply changes based on this feedback
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.
The new MSstatsAnomalyScores roxygen description claims median-based fallback for insufficient quality metrics and parallel-processing support, but neither behavior is visible in the current implementation of MSstatsAnomalyScores / .prepareSpectronautAnomalyInput / .runAnomalyModel. Please either implement these behaviors or adjust the documentation to match what the function actually does.