added precision, recall, f1_score #20
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #20 +/- ##
==========================================
+ Coverage 98.01% 98.11% +0.09%
==========================================
Files 5 5
Lines 101 106 +5
==========================================
+ Hits 99 104 +5
Misses 2 2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks for the PR. This looks really great. Thanks for following the package theme of defining some metrics in terms of other metrics. In addition to my documentation requests and formatting requests, can you also generate documentation for the new functions by running devtools::document()?
R/binary_classification.R
Outdated
|
|
||
| #' Precision | ||
| #' | ||
| #' \code{precision} computes proportion of predicted 1's that are actual 1's |
There was a problem hiding this comment.
Can you change this to, "computes proportion of observations predicted to be in the positive class (i.e. the element in \code{predicted} equals 1) that actually belong to the positive class (i.e. the element in \code{actual} equals 1)?
Thanks
R/binary_classification.R
Outdated
| #' Precision | ||
| #' | ||
| #' \code{precision} computes proportion of predicted 1's that are actual 1's | ||
| #' @export |
There was a problem hiding this comment.
Can you add #' @seealso \code{\link{recall}} \code{\link{fbeta_score}}
R/binary_classification.R
Outdated
| #' Precision | ||
| #' | ||
| #' \code{precision} computes proportion of predicted 1's that are actual 1's | ||
| #' @export |
There was a problem hiding this comment.
Can you also add #' @inheritParams params_binary
R/binary_classification.R
Outdated
| #' predicted <- c(1, 1, 1, 1, 1, 1) | ||
| #' precision(actual, predicted) | ||
| precision <- function(actual, predicted) { | ||
| return(mean(actual[predicted == 1])) |
There was a problem hiding this comment.
Can you use four spaces as the indentation?
R/binary_classification.R
Outdated
| #' @examples | ||
| #' actual <- c(1, 1, 1, 0, 0, 0) | ||
| #' predicted <- c(1, 0, 1, 1, 1, 1) | ||
| #' recall(actual, predicted) |
There was a problem hiding this comment.
Can you add all of the same documentation improvements that were listed for precision?
R/binary_classification.R
Outdated
| #' actual <- c(1, 1, 1, 0, 0, 0) | ||
| #' predicted <- c(1, 0, 1, 1, 1, 1) | ||
| #' recall(actual, predicted) | ||
| f1_score <- function(actual, predicted, beta = 1) { |
R/binary_classification.R
Outdated
| #' @examples | ||
| #' actual <- c(1, 1, 1, 0, 0, 0) | ||
| #' predicted <- c(1, 0, 1, 1, 1, 1) | ||
| #' recall(actual, predicted) |
R/binary_classification.R
Outdated
| #' | ||
| #' \code{f1_score} computes the f1 score | ||
| #' @export | ||
| #' @seealso \code{\link{precision}}, \code{\link{recall}} |
R/binary_classification.R
Outdated
| #' recall(actual, predicted) | ||
| f1_score <- function(actual, predicted, beta = 1) { | ||
| prec = precision(actual, predicted) | ||
| rec = recall(actual, predicted) |
There was a problem hiding this comment.
Can you use the assignment operator <- and use four spaces.
| expect_equal(f1_score(c(1,1,0,0),c(1,1,0,0)), 1) | ||
| expect_equal(f1_score(c(0,0,1,1),c(1,1,1,0)), 2/5) | ||
| expect_equal(f1_score(c(1,1,1,1),c(1,0,0,1)), 2/3) | ||
| }) |
|
thanks for the comments. Let me know your feedback on the F-beta score descriptions I made if you'd like that changed or if you want it to be more specific. If this looks good, still to be done is to update the TeX equations on the main page. |
|
I think that this looks great. I like the descriptions of the If you could add the equations to the README, I'll approve the PR. |
|
Sorry for the delay. I think this does it. |
|
Really excited to see these changes make it into this awesome package. Any thoughts on when this PR will be merged and ideally up on CRAN? |
|
@mthorrell I must have missed the email notification that alerted me to the fact that you pushed changes a month ago! I'm sorry. This all looks good. I'll merge this now. @dimagor I can push to CRAN this weekend. |
|
@mfrasco thank you for merging this. Any luck with the CRAN update? |
|
@dimagor On CRAN now. Sorry for the delay. I missed the email from CRAN that described the changes that I needed to make in order to get it approved. |
|
Fantastic! Thanks a lot!! |
would like to have some type checking since precision, recall and f1 depend on 0 and 1 predictions, but basics should be here. Referencing issue #19.