[datatable-help] Adding the S4-ization patch

Steve Lianoglou mailinglist.honeypot at gmail.com
Tue Feb 22 20:06:14 CET 2011


Hi,

Prompted by the earlier post today from Peter, I'd like to submit my
(simple) S4 patch back into the svn trunk.

This has been on pause for me since we ran into the problem of
shoehorning the s4 tests into the current testing methodology, which
lead me to suggest using testthat for testing ... which I started to
do.

We then spoke about whether or not push all of the tests into
testthat, or just do the s4 for now, and tests in future would use
testthat, and the current ones would stay where they are.

Personally, I'm in favor of (gradually) moving all of the current
tests into testthat -- I've actually submitted  patch to Hadley for
testthat that makes it easier to use the less verbose expect_*
shortform functions with labels so they are more informative. For
example, instead of:

expect_that(1, equals(2), info="Testing numeric equality")

one would do:

expect_equal(1, 2, info="testing numeric equality")

[The current version of testthat doesn't allow to add info/label args
to these short forms -- this is what I submitted a patch for).

So, does this plan seem OK to the devs?

(1) Add my S4 patch to datatable-trunk asap with unit tests using the
current version of test_that.

(2) Once my patch gets into test_that, then I'd volunteer to
(gradually) migrate older tests into this framework (and out of the
data.table manual page).

If we don't want to move the older tests into the test_that framework,
then we can just drop (2).

Thoughts?

Doing this sooner rather than later would be helpful for me so that I
can more easily track latest svn and all of the goodness that has been
added post 1.5.2, but I'm trying not to be selfish ;-)

-steve

-- 
Steve Lianoglou
Graduate Student: Computational Systems Biology
 | Memorial Sloan-Kettering Cancer Center
 | Weill Medical College of Cornell University
Contact Info: http://cbio.mskcc.org/~lianos/contact


More information about the datatable-help mailing list