SCM

Forum: open-discussion

Monitor Forum | Start New Thread Start New Thread
RE: improper use of S3 methods? [ Reply ]
By: Ott Toomet on 2016-02-16 04:20
[forum:42956]
Thanks :-) The tests worked flawlessly.

There are definitely big issues, my favorite is testing. testthat is a good tool, but it is not a substitute for a good design of what and how to test.

Cheers,
Ott


RE: improper use of S3 methods? [ Reply ]
By: Randall Pruim on 2016-02-15 13:00
[forum:42950]
As a test of my ability to use svn within RStudio and my newly acquired developer status, I just committed a change to stdEr(). It would not surprise me if there are similar issues throughout the code base, but I haven't gone looking.

---rjp

RE: improper use of S3 methods? [ Reply ]
By: Ott Toomet on 2016-02-11 04:52
[forum:42909]
I have to look at this. There may be our misunderstandings, leftovers from a previous version, etc in the code.

At one point of time I re-implemented most of it as S4 classes but it never got to the final release. I would love to do it again, maybe even in R6 classes that can handle private members. But we agree very much with Arne to preserve backward compatibility for long time. So, IMHO, the first step in that direction is to implement getters, remove the documentation of returned list members, and document the getters instead.

Comments welcome :-)
Ott

improper use of S3 methods? [ Reply ]
By: Randall Pruim on 2016-02-11 04:24
[forum:42907]
In the code below, I think there is some improper understanding of and use of S3 methods.

1) stdEr.maxLik should only get called when x inherits the maxLik class, so there is no need to test for it.

2) vcov.maxLik() should just be vcov(). The class of x will lead to the proper dispatch of vcov(). Calling methods directly like this is strongly frowned upon.

3) The same goes for coef.maxLik()

> maxLik:::stdEr.maxLik
function (x, eigentol = 1e-12, ...)
{
if (!inherits(x, "maxLik"))
stop("'stdEr.maxLik' called on a non-'maxLik' object")
if (!is.null(vc <- vcov.maxLik(x, eigentol = eigentol))) {
s <- sqrt(diag(vc))
names(s) <- names(coef.maxLik(x))
return(s)
}
return(NULL)
}

Thanks to:
Vienna University of Economics and Business Powered By FusionForge