R package

[MarcHafner]

@jmuhlich will have more comments, but as a start, could you:

• remove (add in .gitignore) the Rproject files and any .R* file which are not necessary to run the code.
• remove the example data and change the test codes to point to the files in the root folder. This will be easier if we ever change these files to not have multiple copies of them.
• similarly, could you ignore any output file that can be reproduced by the example scripts.

thanks, marc

[NicholasClark]

I'll figure out the .gitignore stuff. I don't like the idea of getting rid of the example data though... I'm assuming you mean the objects accessed with the data() function, right? When a user installs the package via install_github(), this is the only way to make that data available. We can't read in the tsv files because we don't know where their R library will be. I don't see much of a problem with having multiple copies of some arbitrary example data.

[MarcHafner]

fair point about the data. I will change the python scripts that generate the example data to make a copy also in the R package folder such that data files will always be identical.

[jmuhlich]

Thanks Nick, this is great. @ArtemSokolov do you have a few minutes to look over the code and make any suggestions about the style, layout, etc? Marc and I don't know R and so we can't really review the code itself.

[ArtemSokolov]

Would be happy to. Can you point me to the R code? I'm having troubles navigating to it from this pull request for some reason.

EDIT: Nevermind, found it.

[jmuhlich]

https://github.com/NicholasClark/gr50_tools/tree/72adb1dd659a00c69dbeff6cc6e158dee525ee8b/SRC/R/GRmetrics

On 06/24/2016 09:50 AM, ArtemSokolov wrote:

Would be happy to. Can you point me to the R code? I'm having troubles navigating to it from this pull request for some reason.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sorgerlab/gr50_tools/pull/22#issuecomment-228350626, or mute the thread https://github.com/notifications/unsubscribe/AAQ6onWE_3_gRhfCMBSP2CoZ8zhS3B6-ks5qO-CMgaJpZM4I4p5O.

[ArtemSokolov]

Everything looks pretty polished. I briefly looked through the code, then downloaded and installed the package and stepped through the example.

I agree that the data examples should stay with the package. That's pretty common in R.

My only comment is to add a write.table( drc_output, ... ) statement to the example to demonstrate how to output the results to a .tsv file. My recommendation is to then add that .tsv file to gr50_tools/OUTPUT to be consistent with the MATLAB and Python outputs that are already there. I briefly checked the first few entries of the MATLAB and Python outputs and they appear to match with what is computed by R.

[NicholasClark]

OK, I added R output files in OUTPUT and examples of write.table in the README as well as in the documentation for the GRfit function.