[datatable-help] Adding the S4-ization patch

[Steve Lianoglou]

Hi Matthew:

On Tue, Feb 22, 2011 at 7:32 PM, Matthew Dowle <mdowle at mdowle.plus.com> wrote:
> Steve,
>
> We definitely need to include your S4 work asap.
>
> My thoughts on the testing are as follows (very likely due to my lack of
> knowledge) :
>
> 1) is test_that the best choice -  what about Runit and SVunit? Which is
> the most acceptable/recommended by r-core? What do other packages use? I
> simply don't know - I've been meaning to investigate. Is there any
> document that weighs up the pros and cons of each? On what basis should
> we choose? R itself doesn't use any of these packages - how do they
> manage?

I suggested test_that because (i) someone (maybe you?) said that a
previous patch was added for RUnit support, but it wasn't clear how to
run tests, so that patch was dropped. test_that allows easy ways to
run tests via R CMD check, but also you can easily test directories
etc. interactively from the R prompt; and (ii) it seems more
intuitive/user friendly to me and is still quite comprehensive with
respect to the types of tests you can check against, eg. normal tests
for equality/identity/inheritance, but also checks for calls that
should throw errors, warnings, or messages.

If anyone strongly favors another testing framework, that's fine too.

> 2) If the call to test.data.table() was moved out of .Rd and into tests
> directory, would the S4 tests then work? If yes, then the choice of
> testing framework may be distinct from the incompatibility problem.

Perhaps.

There was some locked environment issue that was messing things up.

> 3) The tests in the v1.5.3 ?data.table manual page were revised
> recently. They are for users to look at; to learn data.table quickly. At
> least that's what I aimed for. They should be a subset of formal tests
> definitely, but did you really mean to move them out of .Rd ?

I see. Maybe not(?). I think keeping a subset of the usage scenarios
in the Examples section is definitely useful, but last I checked, all
of the tests were in there (sorry if that's not the case -- I'll
double check when I get back to school tomorrow).

I certainly wouldn't propose removing all scenarios/tests from the
example section, though.

> 4) I've used specific numbers to various tests in various places at
> various times: FAQ, FR list, public posts and private emails. If we drop
> the test numbers how will we (I) identify them? There are specific
> numbers I know in my mind which often come up when I change anything -
> test 147 for example. I made a change recently so that test numbers can
> be float, and also independent of the counter of tests passed/failed.

For what it's worth, the numbers/id's could be added to the tests
using the info/label arguments in test_that's expect_* functions.

-steve

>
> Matthew
>
>
> On Tue, 2011-02-22 at 14:06 -0500, Steve Lianoglou wrote:
>> 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