[rkward-devel] correlation plugin [was: another mac issue]

[meik michalke]

hi,

Am Mittwoch 21 Dezember 2011, 12:34:43 schrieb Thomas Friedrichsmeier:
> > p.s.: i added polyserial and polychoric correlations to the correlation
> > matrix plugin. but i was thinking it's probably better to make the radio
> > buttons a dropdown menu now, and move it above the data selection on the
> > first tab, like in the new t-test plugin?
> 
> I think there is a point for moving the method to the first tab, and yes,
> with that many options, a dropdown menu is probably better. I don't think
> it should go above the data selection in this case: The process of
> selecting data is not affected by the selection, so I think the "natural"
> sequence is to select the data, then an appropriate method, here, not the
> other way around.

that depends -- i was also thinking about adding a second variable slot for 
categorial variables for the poly* correlations, then. they can actually be 
provided as numeric, but especially polyserial() needs the first variable to 
be the "real" numeric one and treats the second as categorial, no matter what. 
right now there's no real control for that, especially not transparent check 
if you provided appropriate data at all. the hack with the is.numeric() 
condition is, well, a little ugly. [but of course polyserial() takes ages if 
it correlates two numerics...]

> BTW, it looks to me now, like we have a bug when excluding "whole cases".
> Here, in the calculation of p-values, we still seem to use pairwise
> exclusion, unconditionally (but fortunately, at least we print n,
> correctly). Should we use
>    data <- data[complete.cases (data)]
> for "whole case" exclusion instead? That should also allow us to offer
> "whole case" exclusion for polyserial and polychoric, too, right?

those parts i didn't touch, i think. since there already is a line
  data <- data[complete.cases (data),]
i'm not sure what you mean, exactly. but you're right, i made that available 
for all correlations now.

> The usual minor nitpicks:

as usual quite helpful ;-)

> - Instead of
>    else {
>        t <- NULL
>    }
> and then
>    if (length (t) > 0) ...
> I suggest using
>    else {
>       next
>    }

good one, done.

> - Here, and elsewhere I noted you tend to add empty else-clauses ("else
> {}"), sometimes. Do you feel that helps readability? Or is that just an
> artifact of the rkwarddev-package?

i actually do that on purpose, and yes, i think it helps readability, or more 
precisely, it prevents at least me to misunderstand anything after "if" to be 
intended as its "else" case, which i must admit happend to me in the past.
for the same reason i usually write curly brackets even when they are not 
needed.

e.g., this will work, but one can misinterpret the "else":

 if(cond1) {
   if(cond2)
     foo
 } else bar

while this, i guess, also leaves no doubt what was meant to happen:

 if(cond1) {
   if(cond2) {
     foo
   } else {}
 } else {
   bar
 }

> - In the .xml, the properties polyser, polycho, and polycor are really
> inverted. Either invert the logic, or change the names of the properties, to
> avoid confusion.

right, changed that too.


viele grüße :: m.eik

-- 
dipl. psych. meik michalke
institut f"ur experimentelle psychologie
abt. f"ur diagnostik und differentielle psychologie
heinrich-heine-universit"at d-40204 d"usseldorf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/rkward-devel/attachments/20111221/0db71631/attachment.sig>