MCS code #19
MCS code #19
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
=======================================
Coverage 99.44% 99.44%
=======================================
Files 22 22
Lines 1086 1086
=======================================
Hits 1080 1080
Misses 6 6 |
There was a problem hiding this comment.
Thanks @mmartinoli87 for your code. In order to move forward it would be helpful if you could add some comments, particularly -but not exclusively- explaining what the various functions do even in abstract terms, and perhaps also describing the meaning of the key variables (see e.g., the questions I made on the code in my review).
These comments would be very useful in the transition of the code from R to Python, because they would allow a Python programmer to easily interpret the code without being an expert in R. Moreover, we can then add your comments in the docstrings of the new Python code.
You could add your code comments directly within the "MCS.R" module. Alternatively you can write them here in the chat and I can update the file.
black_it/MCS.R
Outdated
| @@ -0,0 +1,105 @@ | |||
| # Reading the data | |||
|
|
|||
| mData <- read.csv("Data.csv") | |||
There was a problem hiding this comment.
What is the content of the mData matrix? Specifically, I see it has 4 columns but what do those columns represent exactly?
black_it/MCS.R
Outdated
| mData <- read.csv("Data.csv") | ||
| attach(mData) | ||
|
|
||
| fnComplete <- function(x,l=301) { |
There was a problem hiding this comment.
What does this function do? A line of comment would be useful here
black_it/MCS.R
Outdated
|
|
||
| # Building the MCS | ||
|
|
||
| vZ <- 300+4.9*(1:301) |
| vZ <- 300+4.9*(1:301) | ||
|
|
||
| mDist <- NULL | ||
| for (i in 1:16) { |
There was a problem hiding this comment.
What does this for loop do? and the following one? Just two likes of comments could be really helpful
| library(data.table) | ||
| mD <- setDT(mDist)[,list(mean=mean(Dist),var=var(Dist)),by=c("COP")] | ||
|
|
||
| fnTest <- function(vMean,vVar,iN,dA=1) { |
There was a problem hiding this comment.
Comments here would also be useful
| return(list(test=dTest,q=dQuan,p=dPVa,result=dResult)) | ||
| } | ||
|
|
||
| fnElim <- function(vMean,vVar,vIndex) { |
| return(list(mean=vMean[-iM],var=vVar[-iM],indices=vIndex[-iM],elim=vIndex[iM])) | ||
| } | ||
|
|
||
| fnMCS <- function(vMean,vVar,vIndex,dA,verbose=1) { |
There was a problem hiding this comment.
Hi @AldoGl, I committed a commented version of the MCS code
|
Thank you @mmartinoli87 for the new code, the comments are indeed very useful! |
|
Hi, @mmartinoli87: the two notifications you saw about this PR were just a test to verify that black-it repo is correctly configured to collaborate on pull requests from external contributors. Everything went smoothly: you and @AldoGl can safely work together, no action needed on your part. |
|
Thank you @muxator! |
Uploading MCS R code