javadch / rquis Goto Github PK

The current function documentation in R/RQt.R should be improved, following guidelines from Hadley.

Some outstanding issues I see:

• input/output types not clearly specified
• short titles and no long descriptors for some functions
• all the Descriptions (second line) start with the name of the function
• all examples are both ran and not ran; this is unnecessary. Because of the complex nature of some of these functions and issues with pathing, it's fine to have not-quite-valid code listed in \dontrun{}; more helpful, complex examples can be in demos/vignettes (see #18).

It would be wise to enshrine suggested development practices in a file to keep contributors on the same page. This could include things like our git workflow, what commands to use when building or running R CMD check, etc. An example of such a file can be found in ggplot2.

I executed the script non-interactively using

RQt ellisvalentiner$ R CMD BATCH --no-save --no-restore /
    Examples/RQtTest1.R ~/Desktop/RQtLogs/RQtTest1.log

The output from the log file:

R version 3.2.2 (2015-08-14) -- "Fire Safety"
Copyright (C) 2015 The R Foundation for Statistical Computing
Platform: x86_64-apple-darwin13.4.0 (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> library(RQt)
Loading required package: rJava
> library(ggplot2)
> library(scales)
> library(plyr)
> 
> engine <- rqt.getEngine()
> 
> ad <- rqt.getAdapterNames(engine)
> 
> ld <- rqt.loadProcess(engine, "Examples\\processes\\ex1.xqt")
> 
> rn <- rqt.runProcess(engine)
> err <- rqt.getRunReport(engine)
> 
> 
> var2 <- rqt.getVariable(engine, "var2")
Error in .jcall(engine, "Ljava/lang/Object;", "getVariable", variableName) : 
  java.lang.NullPointerException
Calls: rqt.getVariable -> .jcall -> .jcheck -> .Call
R version 3.2.2 (2015-08-14)
Platform: x86_64-apple-darwin13.4.0 (64-bit)
Running under: OS X 10.10.5 (Yosemite)

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] plyr_1.8.3    scales_0.3.0  ggplot2_1.0.1 RQt_1.0       rJava_0.9-7  

loaded via a namespace (and not attached):
 [1] Rcpp_0.12.1      digest_0.6.8     MASS_7.3-44      grid_3.2.2       gtable_0.1.2    
 [6] magrittr_1.5     stringi_0.5-5    reshape2_1.4.1   proto_0.3-10     tools_3.2.2     
[11] stringr_1.0.0    munsell_0.4.2    colorspace_1.2-6
> proc.time()
   user  system elapsed 
  1.766   0.133   1.382 

There's a line in Examples/processes/ex1 which references SOURCE_URI="D:\\Projects\\PhD\\Data\\, a path that anyone but @javadch likely doesn't have. It uses the data_10M file in the directory, which is not currently in the package (not even in extdata).

@javadch I think you brought this file up in either email or some other issue and asked if we should remove it or not because it's a big file, not realizing it's not in there. In the interest of time I recommend we either remove ex1.xqt entirely, or have it use a smaller file. What do you think?

Build check shows the DESCRIPTION file needs updating

* checking DESCRIPTION meta-information ... NOTE
Malformed Description field: should contain one or more complete sentences.
Non-standard license specification:
  What license is it under?

https://www.r-project.org/Licenses/

Per @javadch, this folder needs to be placed in RQt.Rcheck/ after running R CMD check the first time in order to pass the checking process.

Javad, do you mean config/ needs to be in RQt.RCheck/RQt/? If so I can solve this by moving config/ into inst/ so it moved up during the build process. If it really needs to be at the same level as RQt.RCheck/RQt/, rather than inside it, we have a bigger problem; that simply can't be done.

What's really going on with this folder? All I see are *.jar and *.xml files; perhaps the former could go in insta/java and the latter in extdata? If that works, I imagine there's lot's of references to update.

Right now, the best way of checking that JDK_HOME was found/set and that the JDK is >= 1.8 is to run demo("test", package="RQt"). There's gotta be a good way to check configuration without discrete user action; perhaps in .onLoad?

It is hard to set the JAVA_HOME, JRE_JOME, and PATH variables required for the package to function properly. Would be good if it is done transparently.

When I filled out the DESCRIPTION file, I just wrote something in the Title and Description fields without given too much thought. These should be improved, and will likely follow from #9.

@javadch has provided several documents via email which need to be thoroughly read and combined into a single README for this project.

Some things to address:

• what is RQt?
• what's the use-case for RQt, what's compelling about it?
• how do you properly install and configure RQt?
• how do you use RQt?

Once we have an updated master branch, we should use Travis for continuous integration, which will run R CMD check automatically every time master is updated. Once it's working we should also include the appropriate badge on the README.

The Examples/ directory is currently a collection of 4 R scripts which are essentially demos. This should be formalized demos; currently, a user has no good way to run these Examples anyways.

According to Hadley, demos are inferior to vignettes. Not only that, but demos appear to be ran from wherever a user types demo(), which would be a problem for the paths in the xqt scripts (which would themselves be moved to extdata). If it was instead the case that demos run from RQt/demo/ in our case, we could more easily get the paths working and stick with demos for now.

According to the same link, the demos aren't even checked automatically, though I wouldn't be surprised if the CRAN maintainers checked them manually, at least for the first submission of a package.

I'll make a branch and edit things assuming demos are ran from RQT/demo, but again @javadch will have to test for me. If that doesn't work we either need a different way to inject paths into an xqt script, or we need to write a vignette for these long examples.

To reproduce, checkout the new-demos branch, build the package, then run

demo(test, package="RQt")

You'll have to hit enter to start it; it should run "successfully", but the rqt.runProcess call does not return what we want:

> rqt.runProcess(engine)
[1] "Could not read the input stream: /home/cyochum/config/adapters.xml (No such file or directory)"

That's definitely the wrong path to be looking in, but I have no idea how we could tell Java where to look. You can see in demo/test.R I use find.package(), but I don't know how to pass that to Java.

This wasn't caught by R CMD check because that outcome looks successful to R since no error was raised.

I don't know how to fix this, but this is pretty serious. I suspect the way we're using config is not ideal, but I don't know where else to put that stuff. Perhaps as a short-term workaround you can edit the appropriate java functions to take in a directory parameter, and then in R we could hardcode those to find.package("RQt") inside the R functions?

While the naming scheme of rqt.someFunction does clearly indicate the function comes from this package, that naming style has a very OO implication; it looks like we're calling the function rqt on an object of the someFunction class.

A new naming scheme needs to still clearly attribute functions to this package, but in a more modern, standard way.

In reading up on Java+R I came across the last paragraph here. I'm not sure how necessary including Java source is, but I see the XLConnect package just used a README file with a link to another repo which contains the source; perhaps we could do the same here, and just point to https://github.com/javadch/XQt? Does that repo have the source for all *.jar's in inst/java/?

This has been discussed a bit in #4 and #7 ; making a task here for clarity. Completing this should remove both of those warnings from R CMD check output.

In addition to moving the data, lots of references need to be updated from both Examples/ and function docs in R/RQt.R.

The function rqt.getPlot is all commented out at the moment. @javadch, is it important to get this working in v0.1, or should it be removed?

Running R CMD check gives the following:

* checking PDF version of manual ... WARNING
LaTeX errors when creating PDF version.
This typically indicates Rd problems.
* checking PDF version of manual without hyperrefs or index ... OK
* DONE

This means there's some improper documentation code, almost certainly in R/RQt.R, as that's where all the documentation is. This needs to be tracked down, and will resolve the last Warning I see.

From R CMD check:

* checking contents of ‘data’ directory ... WARNING
Files not of a type allowed in a ‘data’ directory:
  ‘FSO2014H.csv.hdr’
Please use e.g. ‘inst/extdata’ for non-R data files

I think the idea here is to show how one can use a separate file to specify headers, but it needs to be organized different in the R package. The suggestion to use inst/extdata may or may not be the best option. See Hadley's guide for more.

There's a NOTE thrown by R CMD check:

* checking Rd line widths ... NOTE
Rd file 'rqt.addScript.Rd':
  \examples lines wider than 100 characters:
     addedScript <- rqt.addScript(engine1, "CONNECTION cnn1 ADAPTER=CSV SOURCE_URI='extdata\\\\' PARAMETERS=delimiter:comma, fileExtension:c ... [TRUNCATED]
     addedScript <- rqt.addScript(engine1, "CONNECTION cnn1 ADAPTER=CSV SOURCE_URI='extdata\\\\' PARAMETERS=delimiter:comma, fileExtension:c ... [TRUNCATED]

These lines will be truncated in the PDF manual.

These should be pretty easy to shorten up.