[GitHub] incubator-zeppelin pull request: R Interpreter for Zeppelin

[Leemoonsoo <gi...@git.apache.org>]

Github user Leemoonsoo commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-156766945
  
    @elbamos 
    
    Thanks for the contribution and 
    I briefly reviewed your codes and here's few comment.
    
    ##### 1. Scala->R invocation
    
    You seem to use rscala and separate connection to make `scala -> r` invocation.
    I think SparkR already have RBackend that support `r -> scala` invocation and the same way we do in PySparkInterpreter, this one way connection can be also used to `scala -> r` without introducing much complexes and additional socket connection.
    
    Using similar technique that PySparkInterpreter does for py4j in R Interpreter will help 
    
     * Much smaller code base while there are no need for additional socket connection beside the SparkR provided one.
     * More consistent way with already existing PySparkInterpreter
    
    
    ##### 2. KnitR Inteprreter
    
    Seems like calling knit2html() is the role.
    In this case, i think it more make sense to make a function and inject into RInterpreter (like z.show() in SparkInterpreter), rather than making separate interpreter.
    
    ##### 3. Author, Copyrights and Maintainer tag
    
    Zeppelin discourages Author tag. as well as Copyright / Maintainer if it is not really necessary.
    Your contribution history will kept in commit history and archived in mailing list.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---