[RkWard-devel] RKWard 0.4.5pre1, 0.4.5 release process

[I. Soumpasis]

2007/1/11, Thomas Friedrichsmeier <thomas.friedrichsmeier at ruhr-uni-bochum.de
>:
>
> Hi!
>
> On Thursday 11 January 2007 19:43, I. Soumpasis wrote:
> > Sending
> >
> plots_cor_graph.tar.gz<?&ik=90971e2fbb&view=att&th=110126b9da3bad04&attid=0
> >.1>containing the two files (xml and php) right now.
>
> A number of nitpicks (but it does look pretty good, overall):
> 1) You put all the R code inside printout (). The point of having
> different
> sections is to provide a little common structure. In this case, the
> definition of the helper functions should go into preprocess (), the call
> to
> rm() should go to cleanup ().


Done.

2) Indentation: The next line after an opening brace ('{') should always be
> indented by one Tab (until the closing '}'). This makes reading the code
> structure much easier. Also, maybe you could replace the spaces that are
> used
> to indent the panel.cor()-block right now with tabs, as well.


You 're right. Done.

3) It would probably be useful to have the "method", and "use" options to
> cor
> and cor.test as well. You can copy the relevant sections from the
> correlation
> matrix plugin. One way to make this work, would be to extend the panel.cor
> function with those two paramenters, then modify the call to pairs to:
>
> pairs(x, lower.panel=panel.smooth, upper.panel=function (x, y) panel.cor(x,
> y, use, method))


I have done it with a different method, don't know if it is ok. I applied
use, method to the cor and cor.test functions. Tested the method and works.
Is this ok or should I try the method proposed?


4) The "digits" and "prefix" parameters to panel.cor are effectively unused,
> currently. I don't really understand, what the prefix parameter is for,
> but
> maybe you could also add a GUI option for the digits parameter (that, or
> remove it to keep the code simple).


I gave an option to the user to select precision (1,2,or 3 digits - Sould we
do something else here?). Removed prefix.

5) You wrap up everything in a function rk.temp.cor.graph (). I'm not
> entirely
> sure, whether I like or dislike this (so far we did not use this). Anybody
> have an opinion already?


  There is not a function that can do this to my knowledge so I made this
one using the example. This is the way I have implemented it in R and just
pasted here. One good thing is that you have to delete only this one
function stored in R (and your data). One other that someone could pass
scripts or functions already written in a very easy way. I don't know.
Should I change that?

6) It's usually a good idea to wrap everything between rk.graph.on () and
> rk.graph.off () in a try () statement, i.e.:
> rk.graph.on ()
> try ({
>         rk.temp.cor.graph (rk.temp.x)

    })

> rk.graph.off ()
> This makes sure that the graphing device gets shut down in case there was
> an
> error during the plot. Which reminds me, that I intended to add this
> everywhere else...



Done.

7) The idea of scaling the text the r is printed in according to the value
> of
> r is definitely interesting, but I think it should be possible to turn it
> off.


Done.

All this may seem like a lot of criticisms, but mostly it's small or
> optional
> stuff, so I hope you don't feel discouraged. The most important thing is
> that
> it works and is useful, and your code works nicely indeed, and is a pretty
> nifty feature! If you need more details/comments, just ask.



Thanks too much for the criticisms, I was waiting for them. I am open and I
am learning, not discouraged at all. Glad you liked it.

Two more things.
1. I added a note with what the asterisks mean.
2. I  believe that the plugin should be placed under the same menu with
correlation matrix, I mean something like this.

Analysis -- Correlation -- Correlation matrix
                                  |
                                  -- Correlation graph


> Regards
> Thomas
>
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share
> your
> opinions on IT & business topics through brief surveys - and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>
> _______________________________________________
> RKWard-devel mailing list
> RKWard-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/rkward-devel
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/rkward-devel/attachments/20070112/2461737d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: plots_cor_graph.tar.gz
Type: application/x-gzip
Size: 1242 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/rkward-devel/attachments/20070112/2461737d/attachment.gz>