<br><br><div><span class="gmail_quote">2007/1/11, Thomas Friedrichsmeier <<a href="mailto:thomas.friedrichsmeier@ruhr-uni-bochum.de">thomas.friedrichsmeier@ruhr-uni-bochum.de</a>>:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

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.

<br><br>A number of nitpicks (but it does look pretty good, overall):<br>1) You put all the R code inside printout (). The point of having different<br>sections is to provide a little common structure. In this case, the<br>

definition of the helper functions should go into preprocess (), the call to<br>rm() should go to cleanup ().</blockquote><div><br>Done. <br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

2) Indentation: The next line after an opening brace ('{') should always be<br>indented by one Tab (until the closing '}'). This makes reading the code<br>structure much easier. Also, maybe you could replace the spaces that are used

<br>to indent the panel.cor()-block right now with tabs, as well.</blockquote><div><br>You 're right. Done. <br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

3) It would probably be useful to have the "method", and "use" options to cor<br>and cor.test as well. You can copy the relevant sections from the correlation<br>matrix plugin. One way to make this work, would be to extend the 

panel.cor<br>function with those two paramenters, then modify the call to pairs to:<br><br>pairs(x, lower.panel=panel.smooth, upper.panel=function (x, y) panel.cor (x,<br>y, use, method))</blockquote><div><br>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?<br> </div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

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

<br>remove it to keep the code simple).</blockquote><div><br>I gave an option to the user to select precision (1,2,or 3 digits - Sould we do something else here?). Removed prefix.<br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

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?</blockquote><div>   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?

<br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">6) It's usually a good idea to wrap everything between rk.graph.on () and<br>

rk.graph.off () in a try () statement, i.e.:<br>rk.graph.on ()<br>try ({<br>        rk.temp.cor.graph (rk.temp.x)</blockquote>    })<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

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

<br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">7) The idea of scaling the text the r is printed in according to the value of<br>

r is definitely interesting, but I think it should be possible to turn it<br>off.</blockquote><div><br>Done.</div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

All this may seem like a lot of criticisms, but mostly it's small or optional<br>stuff, so I hope you don't feel discouraged. The most important thing is that<br>it works and is useful, and your code works nicely indeed, and is a pretty

<br>nifty feature! If you need more details/comments, just ask.</blockquote><div><br><br>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.<br>

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

<br>                                  |<br>                                  -- Correlation graph<br></div>                               <br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

Regards<br>Thomas<br><br><br>-------------------------------------------------------------------------<br>Take Surveys. Earn Cash. Influence the Future of IT<br>Join SourceForge.net's Techsay panel and you'll get the chance to share your

<br>opinions on IT & business topics through brief surveys - and earn cash<br><a href="http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV">http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

</a><br><br>_______________________________________________<br>RKWard-devel mailing list<br><a href="mailto:RKWard-devel@lists.sourceforge.net">RKWard-devel@lists.sourceforge.net</a><br><a href="https://lists.sourceforge.net/lists/listinfo/rkward-devel">

https://lists.sourceforge.net/lists/listinfo/rkward-devel</a><br><br><br><br></blockquote></div><br>