You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by zjffdu <gi...@git.apache.org> on 2017/04/11 10:19:34 UTC

[GitHub] zeppelin pull request #2241: ZEPPELIN-2390. Improve returnType for z.checkbo...

GitHub user zjffdu opened a pull request:

    https://github.com/apache/zeppelin/pull/2241

    ZEPPELIN-2390. Improve returnType for z.checkbox

    ### What is this PR for?
    Currently it is not convenient to access the individual item of the return value of z.checkbox, I would propose to return `Seq` for `SparkInterpreter` and `list` for `PySparkInterpreter`.  This might cause some incompatibility, but I think it is acceptable considering the benefits.  Besides that, before this PR, all the items of checkbox would be checked by default in `PySparkInterpreter` which is inconsistent with `SparkInterpreter`, so I change it to nothing is selected by default in this PR. 
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-2390
    
    ### How should this be tested?
    Unit test is added
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/zjffdu/zeppelin ZEPPELIN-2390

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/2241.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2241
    
----
commit 9bb616d95c6d4714d0574374cd314c2c87b3bfa0
Author: Jeff Zhang <zj...@apache.org>
Date:   2017-04-11T10:09:35Z

    ZEPPELIN-2390. Improve returnType for z.checkbox

----


---
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.
---

[GitHub] zeppelin pull request #2241: ZEPPELIN-2390. Improve returnType for z.checkbo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/zeppelin/pull/2241


---
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.
---

[GitHub] zeppelin issue #2241: ZEPPELIN-2390. Improve returnType for z.checkbox

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/zeppelin/pull/2241
  
    Ok sounds ok to me
    
    



---
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.
---

[GitHub] zeppelin issue #2241: ZEPPELIN-2390. Improve returnType for z.checkbox

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2241
  
    Thanks for review, will merge to master if no more discussion. 


---
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.
---

[GitHub] zeppelin issue #2241: ZEPPELIN-2390. Improve returnType for z.checkbox

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/zeppelin/pull/2241
  
    A bit concerned with breaking API changes like this but I guess we have never defined the support or versioning plan on the API?
    
    Aren't we breaking the selected default item for checkbox by initializing
    defaultChecked to empty?
    
    It was default to first item in checkbox
    Now is default to nothing
    



---
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.
---

[GitHub] zeppelin issue #2241: ZEPPELIN-2390. Improve returnType for z.checkbox

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2241
  
    Before this PR It was not default to first item in checkbox, it is default to all items. The reason I change it to nothing is to make it consistent with scala implementation. 
    
    Right, this would break the existing API, but I think it is worthwhile. I have several following PRs to improve `ZeppelinContext` and hope to stabilize it. IMO `ZeppelinContext` would play an important role for extending the rich functionality of zeppelin.



---
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.
---

[GitHub] zeppelin issue #2241: ZEPPELIN-2390. Improve returnType for z.checkbox

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2241
  
    @Leemoonsoo @felixcheung Please help review. 


---
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.
---

[GitHub] zeppelin issue #2241: ZEPPELIN-2390. Improve returnType for z.checkbox

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/2241
  
    LGTM


---
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.
---

[GitHub] zeppelin issue #2241: ZEPPELIN-2390. Improve returnType for z.checkbox

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2241
  
    Will merge to master if no more discussion. 


---
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.
---