-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgraded to new script library that provides foundation for R key words #8707
Conversation
@rdstern This PR is ready for testing. I replaced the RScript library with the new library that I've been working on. It is a different library but the version in this PR should provide the same functionality as RScript. This version intentionally does not contain new features. I first wanted to ensure that it is 100% compatible with R-Instat. Please could you test that the script window works the same as before, especially in relation to:
This PR doesn't add new functionality but it's important because subsequent versions of this library will add R key words to the script window. |
@lloyddewit exciting and will start testing today! |
@lloyddewit my first example is working - and your ctrl-enter (initially) seems fine - and, and, and I now have the cursor visible, so already have a bonus! Though perhaps I am coming down to earth now. Mayb we always had that? When I press Run there is no cursor. But it is there after ctrl enter. Oh well - live and learn. On we go! I am taking the opportunity to explore new scripts at the same time. I am happy with the ease that Insert dialog in the script window gives access to the scripts from the packages. I am wondering where this should should figure in the Help, and should there be another ordinary menu item somewhere that gives access to this dialog? agricolae is an important package. It is in R-Instat and will continue to have features that will only run from a script. So I am starting there:
The problem - if it is one - is in lines 15 (starting plot and the first line of the plot) to line 29 which is where the plot is finished. Here is the plot at the end: It is quite nice and using the ordinary R graphs. I am not sure there is a problem here, i.e. is there a way you could know that these are all a multi-line command? I think you may have the same problem in RStudio if you run line-by-line? So, @lloyddewit I suspect this may not be your problem? If that is the case there are 3 (at least) follow-up questions. I haven't finished testing, but this is the end of my first comment. |
Here are 3 examples. First is the alpha designs still from agricolae package. It seems fine with Looking at the code belowI wonder if there are examples with more complex stuff inside square brackets?
Second is carolina also from agricolae. It gives an odd message caused by lines like this :
Third is an example with a single line that doesn't run. I think it needs your new stuff, before it can. Interesting though that it is the first example I have found where the problem is not detected at the top - when I try the first Run. Is that because the problem is a single line? It is here:
|
@rdstern Thank you for your testing and comments. Your comment from 28 Dec:
|
@rdstern All these examples are brilliant, thank you Your comment from 29 Dec:
|
@lloyddewit I added a third example above - after you saw the first 2. I have continued, partly as a test of the new script-sysem and it seems to be holding up very well. So here is the opposite, namely code that worked and I didn't expect it to"
In the code as downloaded the graphs are commented out so I ran the last-but-one line, with the last line (which starts with a comma!) commented out, and it ran fine. Then I uncommented (Is that a word?) the last line and it correctly included it in the statement. Neat. I don't think we could do that earlier? By the way I have run quite a number to also check what happens with multiple files So far, so good. |
@lloyddewit perhaps this example needs to wait and we can try once the loops are in. It complains even with the replacement method, but then seems to run ok? It is just the lines at the end that cause the problem.
|
@lloyddewit I have been exploring the scripts in agricolae and am now up to about 50. It has done well so far - including lines with comments at the end, and also lines with multiple statements per line. Here is another example with a one line code that means it doesn't yet work.
|
@lloyddewit and now for something completely different. In the tokenizers package (which includes the text of Moby Dick which I was looking for!) one example was giving me some problems for a new reason, namely it includes
I continue here, but now think it might be ok, and an "interesting" example. I added: song <- as.data.frame(song) Save data frame(s) "song"data_book$import_data(data_tables=list(song=song)) So it imports into R-Instat. It seemed to give me just the last line But I now find that it is actually all there. I used the Edit > Wordwrap dialog, and found it all! So far so good. Now I wanted to illustrate that when data are messy initially, then ananalysis could start in R - with the script - and only move into R-Instat when it is a bit less "extreme". This is a trivial example. |
@rdstern All this testing is wonderful, thank you.
I understand that you ran
Yes, this script should work with the new library when key words are implemented. As you say, it runs in the non-strict method but it displays the error message twice and does not display the
This script fails because it contains the |
…layed in output window
a) I first tried the lateblight, which is a useful function to have. It now - I think - runs perfectly for the current code. So where there is a loop is complains that it will have to use the "reserve" method. This now runs with no complaint to give this graph: b) You mentioned this code with the closing bracket missing:
I hadn't noticed that aspect and am very happy that code with incorrect number of brackets will give an error. I assume, with the code above, that it would check that the statement continues to the next line, so will be happy with the statement as a whole. (In my unfair test above I had commented out the second line!) c) The alpha design example is much better, but there is still an "interesting" point. Here is the code:
One example is in the last 2 lines. If I run the d) The carolina example has the same - or similar problem: If I run those 2 lines together, as shown, then it complains and then lists the 2 lines of code, with no further complaint - but there is also no output. |
@lloyddewit happy New Year. I here have a code I really would like to run. It fits perfectly with the current presentation, where I am introducing your scripts stuff. It is the one function in the janeaustenr package. Here is the code as supplied:
I took the comment away from the 2 lines with I can't seem to get it to run? |
@rdstern Thanks for the extra feedback and happy new year to you too. :)
I experimented with this. For me:
This script should work with the new library when key words are implemented so I suggest we don't spend too much time investigating the non-strict mode (we know it will never work perfectly and should be obsolete at one point anyway).
I could not recreate this problem but I anyway fixed the
The
|
@lloyddewit should I be testing this? |
@rdstern No, not yet. I'll let you know when it's ready. |
@rdstern
|
@lloyddewit will do. But you do have access to the Nightingdale package and code, if you did also wish to look yourself. It is in the |
@rdstern Thanks for the hint. I tested the Nightingale script. |
@lloyddewit agreed it is just the coord_polar (with the facets) that seems to give a problem. I would like to know what happens in RStudio.? Your code above runs, but if you then have a line with just cxc to display the plot, then you get the same error message as before. Great that the rest runs nicely. I'll keep checking and with some other scripts! |
I have found the histdata sets havew useful code. So I am trying them in turn. Break to be with grandchildren, but these seem interesting datasets for the analyses that are provided with them. |
@lloyddewit it was going so well too. Let's be clear on the problem. This runs fine, i.e. doing the 3 lines together and gives me a n ice plot. The problem comes from running a line at a time. |
@rdstern Thank you for finding this bug. The problem seems to be in the output system. I can make R-Instat hang with the steps below. It's possible that R-Instat's memory is corrupted. I don't think we should try and fix this problem in this PR. I suggest that we move this into a separate issue. @Patowhiz has expertise with the output system and he is probably the best person to assign to this. I can recreate the issue with these steps:
|
@rdstern Thank you for all the wonderful testing you did. I understand that the script window in this PR now handles more scripts than the current master; there is also no known regression. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lloyddewit that would be great. @N-thony could you also check and then merge. I note that @N-thony change with the windows is merged already and is looking nice!
Antoine - as soon as you like.
@rdstern Please ignore this PR for now, it's just to test if converting the RScript library to C# created any compatibility issues.
Thanks