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 2018/01/15 06:57:13 UTC

[GitHub] zeppelin pull request #2730: ZEPPELIN-3166. R plotting resolution and image ...

GitHub user zjffdu opened a pull request:

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

    ZEPPELIN-3166. R plotting resolution and image width is not proper

    ### What is this PR for?
    Change the resolution and image width, because the previous default value is not proper.
    
    By adding `fig.retina = 2` for high resolution. And use `40%` as default image width. 
    
    
    ### What type of PR is it?
    [ Improvement ]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-3166
    
    ### How should this be tested?
    * Manually verfifed
    
    ### Screenshots (if appropriate)
    
    Before
    
    ![screen shot 2018-01-15 at 2 27 01 pm](https://user-images.githubusercontent.com/164491/34930418-46680e38-fa04-11e7-9b73-b12bd61986f2.png)
    
    After 
    
    ![screen shot 2018-01-15 at 2 27 11 pm](https://user-images.githubusercontent.com/164491/34930430-509204e0-fa04-11e7-927c-c9b4982f8373.png)
    
    ### Questions:
    * Does the licenses files need update?
    * Is there breaking changes for older versions?
    * Does this needs documentation?


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

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

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

    https://github.com/apache/zeppelin/pull/2730.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 #2730
    
----
commit a8c092f17f62ffac5ac16c131966fa89964278dd
Author: Jeff Zhang <zj...@...>
Date:   2018-01-15T06:53:54Z

    ZEPPELIN-3166. R plotting resolution and image width is not proper

----


---

[GitHub] zeppelin issue #2730: ZEPPELIN-3166. R plotting resolution and image width i...

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

    https://github.com/apache/zeppelin/pull/2730
  
    Agree, but the default will always find case where it will not work. For this image size / resolution, there may be more failure than success I am afraid. To be tested with various plots, screen resolutions and paragraphs width to measure this.


---

[GitHub] zeppelin issue #2730: ZEPPELIN-3166. R plotting resolution and image width i...

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

    https://github.com/apache/zeppelin/pull/2730
  
    Thanks @echarles  will merge if no more comments


---

[GitHub] zeppelin issue #2730: ZEPPELIN-3166. R plotting resolution and image width i...

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

    https://github.com/apache/zeppelin/pull/2730
  
    GTM


---

[GitHub] zeppelin issue #2730: ZEPPELIN-3166. R plotting resolution and image width i...

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

    https://github.com/apache/zeppelin/pull/2730
  
    @felixcheung @echarles Could you help review it ? Thanks


---

[GitHub] zeppelin pull request #2730: ZEPPELIN-3166. R plotting resolution and image ...

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

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


---

[GitHub] zeppelin issue #2730: ZEPPELIN-3166. R plotting resolution and image width i...

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

    https://github.com/apache/zeppelin/pull/2730
  
    Oops, never realize there's such magic, seems no document mention that. But we should not rely on these kinds of magic for most of users. We should provide a default value for most of users that no need of any extra overhead no matter it is configuration or such kind of magic.  Thoughts ?


---

[GitHub] zeppelin issue #2730: ZEPPELIN-3166. R plotting resolution and image width i...

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

    https://github.com/apache/zeppelin/pull/2730
  
    `fig.retina` sounds like a good property to play with. However, fixing `out.width` to an arbitrary value may give good result for the example you give, but did you try with other plots?
    
    Showing nice-looking plots generated by knitr depends on your screen resolution and browser actual window width.
    
    We had before (I hope it is still there) the ability to force the image width passing a json after the magic keyword
    
    ```
    %spark.r {"imageWidth": "400px"}
    ```
    
    If you use that `imageWidth` property and push a bit more `fig.retina` to 3 or 4, could it be that most cases will be covered?


---

[GitHub] zeppelin issue #2730: ZEPPELIN-3166. R plotting resolution and image width i...

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

    https://github.com/apache/zeppelin/pull/2730
  
    @echarles I am fine to keep image width as 100%. But setting `fig.retina=2` is necessary, otherwise the image displayed is so unclear.  See below comparison (use both ggplot2 and R base plotting)
    
    Before
    
    ![screen shot 2018-01-16 at 11 28 33 am](https://user-images.githubusercontent.com/164491/34970632-6e71cc50-fab0-11e7-8de9-f2ab7c52c29e.png)
    
    After
    
    ![screen shot 2018-01-16 at 11 28 39 am](https://user-images.githubusercontent.com/164491/34970637-7240e000-fab0-11e7-8743-af958ee99c16.png)
    
    



---