Feature spec fix #93
Feature spec fix #93
Conversation
| @@ -66,7 +66,7 @@ SkylinetoMSstatsFormat = function( | |||
| score_threshold = qvalue_cutoff, | |||
| direction = "smaller", | |||
| behavior = "fill", | |||
| fill_value = 0, | |||
| fill_value = NA_real_, | |||
There was a problem hiding this comment.
Why is the fill_value NA_real_ here but NA for Spectronaut?
There was a problem hiding this comment.
idk! Just following the behavior from further up in the Skyline code.
Apparently:
"In R, NA_real_ is a typed missing value that represents a double, or value with a decimal point, that is missing. NA is the general symbol used to represent missing values in R, and R automatically converts NA to the appropriate typed missing value for the target vector. "
I'm guessing this really doesn't have an impact? Maybe we just go with NA?
@mstaniak do you have a reason to use NA_real_?
There was a problem hiding this comment.
Some packages (for example dplyr-related) check data types very strictly, including NA types. It should be NA_real_ everywhere
| @@ -65,6 +65,8 @@ SpectronauttoMSstatsFormat = function( | |||
| score_filtering = list(pgq = pq_filter, | |||
| psm_q = qval_filter), | |||
| columns_to_fill = list("IsotopeLabelType" = "L")) | |||
| input[, Intensity := ifelse(Intensity == 0, NA, Intensity)] | |||
There was a problem hiding this comment.
Is there any reason we only want to do this with Spectronaut and Skyline but not other converters? i.e. wondering if this could be part of the MSstatsPreprocess function.
There was a problem hiding this comment.
Ah yes we could do this for others, but I am guessing it's not an issue for them? This would require a closer look at the actual data.
There was a problem hiding this comment.
Sounds good, fine with staying within Spectronaut and Skyline first and doing a deeper investigation later (maybe as part of the benchmarking work with Anshuman)
Motivation and Context
Skyline and Spectronaut converters included zeroes instead of NAs which was messing with summarization. Zeroes were being treated as real values. These zeroes have been replaced with NAs for the time being.
Going forward a deeper look into MNAR vs MCAR missingness is needed.
Changes
Testing
Nah
Checklist Before Requesting a Review