Add support for check = 'equal', 'identical', 'equivalent'

[harvey131]

Add support, tests, and documentation for the check argument to microbenchmark to allow it to be a string 'equal', 'identical', or 'equivalent'.

The user will not have to create a separate "my_check" function for common checks.

[joshuaulrich]

I think this is a great suggestion, thanks! A few notes:

• Please keep functional and non-functional (e.g. whitespace) changes in separate commits. Combining them makes the diff larger and distract from the changes that need to be reviewed. That said, please avoid making whitespace-only changes. ;-)
• Please explain all changes in the commit message. You made two changes to unit tests that are not related to this feature, but did not explain why.
• Use a branch as the base for your pull request. Basing your PR on 'master' can make it difficult for you to update your fork if your pull request isn't incorporated verbatim (e.g. the commits are squashed into a single merge commit).

If you give me push access to your PR, I can make these changes and merge. Thanks again!

[harvey131]

I made the changes to the two unrelated test cases because they were not passing, and I think they were not correct. Do they pass for you? They are not related to the pull request.

As a separate proposal, would you be open to accepting a pull request with the existing tests converted a testthat file, so the tests can be run within an rstudio package easily?

[joshuaulrich]

No tests have been changed since the last CRAN release, and all checks pass on CRAN. I haven't run them recently, but they must have worked for me before I submitted. I don't know why they don't work for you, but I assume it's a problem with RStudio and/or dectools. I would not accept a PR that used testthat (or any package from the tidyverse). I'm annoyed enough with the behavioral changes in roxygen2. And the first thing I did as maintainer was remove the hard dependence on ggplot2 and the ~40 packages it requires. Sorry for the bit of a rant. It's nothing you did... I use RUnit for all the other packages I maintain. It would be straight forward to convert this package to use it instead. I didn't convert this package to use RUnit because there were only a few tests and the current solution works.

[harvey131]

OK, I don't see how this test should work; it should expect an error.

test_unit_int <- function()
{
out <- try(print(microbenchmark(NULL, unit=4)), silent = TRUE)
stopifnot(!inherits(out, "try-error"))
}
test_unit_int()

microbenchmark(NULL, unit=4)
Error in match.arg(unit) : 'arg' must be NULL or a character vector

[joshuaulrich]

It looks like I was wrong. That test is incorrect, and does error. I don't understand why it doesn't cause an error with R CMD check. I'll get RUnit set up so there can be some confidence in the tests.

[joshuaulrich]

I'm not sure why GitHub decided to close this... I force-pushed my updated changes to your repo before merging, so you wouldn't haven't to deal with upstream differences with master.

So, despite this saying it's "closed", it really is merged.

[harvey131]

Would it be possible to push this updated revision onto cran? Thanks!

[joshuaulrich]

Yes, thanks for the reminder! I've added it to my to-do list for this weekend.

[joshuaulrich]

microbenchmark 1.4-7 is on its way to CRAN.