You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by eddies <gi...@git.apache.org> on 2016/02/19 10:07:57 UTC
[GitHub] incubator-zeppelin pull request: Leaflet support, based PR 152
GitHub user eddies opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/728
Leaflet support, based PR 152
### What is this PR for?
An update/follow-on for PR 152 that is limited to Leaflet support. There doesn't appear to have been any new movement on PR 152 since August 2015, so this PR updates Madhuka's work against master, but without any of the data validation or other non-Leaflet work.
### What type of PR is it?
Feature
### Todos
N/A
### Is there a relevant Jira issue?
ZEPPELIN-157
### How should this be tested?
Recreate Madhuka's example, e.g. use the following to populate a cell:
https://gist.github.com/eddies/f20241e161aa2bbbb788
Then, in a new cell
```
%sql
select * from myMap
```
There should a new chart selector rightmost that will render the data w/ leaflet. You may need to clean your browser cache initially.
### Screenshots (if appropriate)
See Madhuka's original PR: https://github.com/apache/incubator-zeppelin/pull/152
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? Probably. It wasn't clear to me if PR 152 was going to move ahead in a new, Leaflet-only incarnation as suggested in the comment thread. But I needed Leaflet support, so went ahead with my own fork. I'm happy to work on documentation for this, but not sure if a sample notebook tutorial is what's desired (e.g. maybe something under the Display System docs is preferable?). If so, I'd just stick with something like the sample notebook in PR 152.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/eddies/incubator-zeppelin leaflet
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/728.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 #728
----
commit e45bc2313952785b28547b8ec7542587ed56adec
Author: Edwin Shin <ed...@yourmediashelf.com>
Date: 2016-02-19T08:25:37Z
initial leaflet support, based on http://madhukaudantha.blogspot.sg/2015/08/introducing-new-chart-library-and-types.html
----
---
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] incubator-zeppelin pull request: Leaflet support, based on PR #152
Posted by randerzander <gi...@git.apache.org>.
Github user randerzander commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-192782294
this looks awesome. maps/heatmaps are one of the most asked for features whenever I demo Zeppelin.
With this as a base, how difficult would it be to also include geojson, or have query results auto-translated into geojson map features?
---
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] incubator-zeppelin pull request: Leaflet support, based on PR #152
Posted by Madhuka <gi...@git.apache.org>.
Github user Madhuka commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-193616050
@corneadoug : #765 is contain the basic map feature which is in #152. #765 all checks have passed and no build failures.
@eddies : Heatmaps is nice addition for the basic map visualization, This PR ( #728 after fixing this build failure) can easily lay on top of #765
---
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] incubator-zeppelin pull request: Leaflet support, based on PR #152
Posted by eddies <gi...@git.apache.org>.
Github user eddies commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-190635289
FYI, 18b4f55 is failing CI with the following:
```
[ERROR] bower leaflet.heat#v0.2.0 ECMDERR Failed to execute "git ls-remote --tags --heads git@github.com:Leaflet/Leaflet.heat.git", exit code of #128 Warning: Permanently added the RSA host key for IP address '192.30.252.131' to the list of known hosts. Permission denied (publickey). fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists.
[ERROR]
[ERROR] Additional error details:
[ERROR] Warning: Permanently added the RSA host key for IP address '192.30.252.131' to the list of known hosts.
[ERROR] Permission denied (publickey).
[ERROR] fatal: Could not read from remote repository.
[ERROR]
[ERROR] Please make sure you have the correct access rights
[ERROR] and the repository exists.
```
Unfortunately, leaflet.heat does not support bower (the maintainer has steadfastly rejected any pull requests to include a bower.json). So I explicitly added the git url. But it looks like the host resolution on Travis is unhappy with the address it's getting for github.com.
---
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] incubator-zeppelin pull request: Leaflet support, based on PR #152
Posted by eddies <gi...@git.apache.org>.
Github user eddies commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-193123978
@corneadoug thank you for offering to look into the CI failure. I just successfully re-ran `mvn clean package -Pspark-1.6 -Phadoop-2.6` on this branch locally. If I'm reading the travis log correctly, the CI errors are occurring because of:
```
ERROR org.apache.zeppelin.AbstractZeppelinIT:173 - Exception in SparkParagraphIT while testSpark
org.openqa.selenium.TimeoutException: Timed out after 60 seconds waiting for org.apache.zeppelin.AbstractZeppelinIT$1@76a123fb
```
---
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.
---
Re: [GitHub] incubator-zeppelin pull request: Leaflet support, based
on PR #152
Posted by madhuka udantha <ma...@gmail.com>.
@*randerzander: PR 152 will re-based to master soon.*
On Mon, Mar 7, 2016 at 9:23 AM, randerzander <gi...@git.apache.org> wrote:
> Github user randerzander commented on the pull request:
>
>
> https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-193081840
>
> @corneadoug not sure about status of #152 , but this PR adds
> significant additional functionality (heatmaps). since this PR is based on
> @Madhuka's work, perhaps both he and @eddies can be given credit for it, if
> they're both agreeable to that approach?
>
> The CI failure looks unrelated to the changes in this PR. Who can help
> resolve them and get these features moving?
>
> Personally I would like to begin working on adding geojson and more
> geo features, but need one of either this PR, or #152 to be completed first.
>
>
> ---
> 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.
> ---
>
--
Cheers,
Madhuka Udantha
http://madhukaudantha.blogspot.com
[GitHub] incubator-zeppelin pull request: Leaflet support, based on PR #152
Posted by randerzander <gi...@git.apache.org>.
Github user randerzander commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-193081840
@corneadoug not sure about status of #152 , but this PR adds significant additional functionality (heatmaps). since this PR is based on @Madhuka's work, perhaps both he and @eddies can be given credit for it, if they're both agreeable to that approach?
The CI failure looks unrelated to the changes in this PR. Who can help resolve them and get these features moving?
Personally I would like to begin working on adding geojson and more geo features, but need one of either this PR, or #152 to be completed first.
---
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] incubator-zeppelin pull request: Leaflet support, based on PR #152
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-193108046
I can take a look at the CI failure.
I think the heatmap is a nice addition to the basic map. Maybe it could be toggled as an option instead of a different visualization button (just like the options in line chart)
If #152 gets done this week, I think we can rebase @eddies PR on top of it.
If #152 doesn't get done, we would go with @eddies all together.
So, If you want to get started on geojson, it could be best to base your branch on this PR
---
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] incubator-zeppelin pull request: Leaflet support, based on PR #152
Posted by eddies <gi...@git.apache.org>.
Github user eddies commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-193063942
@randerzander I spent a chunk of time last week playing w/ geojson support with pretty mixed results.
I probably sank the most time tracking down an annoying CSS issue that was preventing the SVG layers from rendering properly within Zeppelin. But the real difficulty for me was finding a way to pass in arbitrary geojson. I think supporting the transformation of query results into geojson features is doable, but I still think you need the ability to pass in your own geojson (e.g. a shapefile you've converted to geojson) for it to be a useful feature in Zeppelin and it was the latter that I ultimately gave up on (for the time being).
---
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] incubator-zeppelin pull request: Leaflet support, based on PR #152
Posted by eddies <gi...@git.apache.org>.
Github user eddies commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/728#issuecomment-193127569
> I think the heatmap is a nice addition to the basic map. Maybe it could be toggled as an option instead of a different visualization button (just like the options in line chart)
I agree. In fact, what I really would want to see is an implementation of graphOptions that would, at a minimum, let the user select the lat/lon columns rather than the hard-coded column index assumptions.
But I figured we could call that a future enhancement request, rather than let this (or the original PR) languish any longer....
---
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.
---