Skip to content

ZEPPELIN-157: Adding Map Visualization for Zeppelin - #152

Closed
Madhuka wants to merge 31 commits into
apache:masterfrom
Madhuka:milestone05-map
Closed

ZEPPELIN-157: Adding Map Visualization for Zeppelin#152
Madhuka wants to merge 31 commits into
apache:masterfrom
Madhuka:milestone05-map

Conversation

@Madhuka

Copy link
Copy Markdown
Contributor
  • Listing Map charting Libraries
  • Checking compatible license
  • Adding chart library (leafletjs)
  • Adding new button for chart mapping
  • Loading GIS Mapp in Zeppelin
  • Removing the setting for mapping chart
  • Adding sample data set (loading)
    • Get the sample GIS data and read it
    • Add it as in tutorail, where it can be run from the clean environment
  • Generalization data validation model
    • Add 'validator' for the Map graph input format as service
  • Test for this

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

Map Visualization for Zeppelin

screenshot from 2015-07-13 00 18 19

@vgmartinez

Copy link
Copy Markdown

Hi @Madhuka....
when you try to resize the paragraph the map does not update to the new size ... I have seen that you have:
<leaflet width="100%" height="480px"></leaflet>
It is possible to do dynamic

thanks for you contributions

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

@vgmartinez

Thanks, Yes, I will make it dynamic

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

Add it as in tutorial, where it can be run from the clean environment

screenshot from 2015-07-20 19 04 19

Loading sample GIS Data for tutorial
screenshot from 2015-07-20 18 55 52
screenshot from 2015-07-20 19 53 25

Markers looks like (static look)
screenshot from 2015-07-20 19 54 08

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

Due to -len it is breaking

@Madhuka
Madhukaforce-pushed the milestone05-map branch 4 times, most recently from f4aaa1c to cd15db8CompareJuly 23, 2015 09:13
@Madhuka

Copy link
Copy Markdown
ContributorAuthor

Loading sample data from internet

screenshot from 2015-07-24 23 11 13

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

In Data validatorservices will return error status in boolean true or false. And also it will return error msg with explanation which data record is not validated under data schema.

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

Data validator message will contain {'error':true/false,'msg':'error msg'}.
Data schema can be set as config file.
Data validator is expose as service and it is extendable (factory model).

Here is current screen shoot of PR.
screenshot from 2015-07-26 10 44 47

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rowCheck returns a boolean, however you never use its output. is it a bug?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed,
with setError()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how it addresses the issue. Could you elaborate?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Madhuka?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixcheung :
With updated code basicCheck[1] function return bool and and it is used in checkData()[2].

rowCheck() do not return any bool at now. When ever you found validator issue you can call new function setError();. It will handle it by making errorStatus to true. I will added this function for documentation too. So I will be easy for new developers who will writting new data valitors.

[1] https://github.com/Madhuka/incubator-zeppelin/blob/milestone05-map/zeppelin-web/src/components/data-validator/data-validator-factory.js#L59
[2] https://github.com/Madhuka/incubator-zeppelin/blob/milestone05-map/zeppelin-web/src/components/data-validator/data-validator-factory.js#L32

@Madhuka
Madhukaforce-pushed the milestone05-map branch 4 times, most recently from e27b6b6 to 9a5ce12CompareAugust 12, 2015 04:08
@Madhuka

Copy link
Copy Markdown
ContributorAuthor

screenshot from 2015-08-15 20 40 30
Test are added on data validation.

PR covers all the chart type data models in zeppelin.

@eranwitkon

Copy link
Copy Markdown

Hi,
I tried testing your PR (152) but I could not see the map icon on the output page.
selection_016

Did I miss something?

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

I don't think you missed anything if you have built it correctly. Please clear the browser cache.

@eranwitkon

Copy link
Copy Markdown

Cool that did the trick.
Now that I can run the code I found this behavior :

Try the following code:
%sql
select Name from myMap
where Country = "${Country='United States'}"

The data output of this one have only one column - name but when I press
the map icon I get it to show the data as before. (other view respect the
select and present it as expected)
Eran

On Sun, Aug 16, 2015 at 1:36 PM madhuka notifications@github.com wrote:

I don't think you missed anything if you have built it correctly. Please
clear the browser cache.


Reply to this email directly or view it on GitHub
#152 (comment)
.

@corneadoug

Copy link
Copy Markdown
Contributor

The Build runs multiple time on the CI

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

yes, Build has run multiple times and final build was successful according to log files. But there is still one failing check.

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

The Travis CI is only check first Build only, isn't it? First build test failure which is bind to CI and In next build It was fine. According to log second build (final build) says "BUILD SUCCESS" in the log.

[INFO] Reactor Summary:
[INFO]
[INFO] Zeppelin ........................................... SUCCESS [ 6.055 s]
[INFO] Zeppelin: Interpreter .............................. SUCCESS [ 3.867 s]
[INFO] Zeppelin: Zengine .................................. SUCCESS [ 2.659 s]
[INFO] Zeppelin: Spark .................................... SUCCESS [ 35.690 s]
[INFO] Zeppelin: Markdown interpreter ..................... SUCCESS [ 0.699 s]
[INFO] Zeppelin: Angular interpreter ...................... SUCCESS [ 0.316 s]
[INFO] Zeppelin: Shell interpreter ........................ SUCCESS [ 0.315 s]
[INFO] Zeppelin: Hive interpreter ......................... SUCCESS [ 3.282 s]
[INFO] Zeppelin: Apache Geode interpreter ................. SUCCESS [ 7.297 s]
[INFO] Zeppelin: PostgreSQL interpreter ................... SUCCESS [ 0.413 s]
[INFO] Zeppelin: Tajo interpreter ......................... SUCCESS [ 1.088 s]
[INFO] Zeppelin: Flink .................................... SUCCESS [ 4.955 s]
[INFO] Zeppelin: Apache Ignite interpreter ................ SUCCESS [ 0.680 s]
[INFO] Zeppelin: Lens interpreter ......................... SUCCESS [ 2.181 s]
[INFO] Zeppelin: Cassandra ................................ SUCCESS [ 6.649 s]
[INFO] Zeppelin: web Application .......................... SUCCESS [ 37.351 s]
[INFO] Zeppelin: Server ................................... SUCCESS [ 18.031 s]
[INFO] Zeppelin: Packaging distribution ................... SUCCESS [ 26.737 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:39 min
[INFO] Finished at: 2015-08-17T05:10:28+00:00
[INFO] Final Memory: 56M/360M
[INFO] ------------------------------------------------------------------------
travis_time:end:2f3eec76:start=1439788067730573331,finish=1439788229232148013,duration=161501574682
�[0K

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

CI Build is test with PR[1], It contain data validations (fully) and leaflet support with map and tutorial with squashing commits

[1] #238

@eranwitkon

Copy link
Copy Markdown

Did you see my comment from 5 days ago?

Now that I can run the code I found this behavior :

Try the following code:
%sql
select Name from myMap
where Country = "${Country='United States'}"

The data output of this one have only one column - name but when I press
the map icon I get it to show the data as before. (other view respect the
select and present it as expected)
Eran
בתאריך יום ו׳, 21 באוג׳ 2015, 21:17 מאת madhuka notifications@github.com:

CI Build is test with PR[1], It contain data validations (fully) and
leaflet support with map and tutorial with squashing commits

[1] #238#238


Reply to this email directly or view it on GitHub
#152 (comment)
.

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

@bzz : ZeppelinIT which is failed in CI run fine in local environment. I have re-base (update) the PR. is there any more things to do for this PR?


T E S T S

Running org.apache.zeppelin.ZeppelinIT
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.075 sec - in org.apache.zeppelin.ZeppelinIT

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

[INFO]
[INFO] --- scalatest-maven-plugin:1.0:test (test) @ zeppelin-server ---

@friedhelmvictor

Copy link
Copy Markdown

Map Visualization for Zeppelin would be a great addition!
I've tried the code and ran into several problems.

I don't think it's good to select the lat and lon statically like so:
https://github.com/Madhuka/incubator-zeppelin/blob/milestone05-map/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js#L866
It would be nice if they were detected automatically. Maybe if there are columns lat, latitude, lon, long or longitude, pick them?

The same goes for the key:
https://github.com/Madhuka/incubator-zeppelin/blob/milestone05-map/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js#L863
If a key exists twice, it obviously breaks. Key and message are the same though, but I may be interested in the same message at different locations.

There should be a settings drop down like for the other chart types where this can be configured. With the current version, that doesn't seem to work. (Only for me?)

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

@datagiraffe : thanks for the interest

  1. Current zeppelin is written on order_of_key level. It is not written to detected automatically at the moment.
  2. key is local variable and it is not twice. And all the variable name can have issue. That is the reason 'jshint' in zeppelin. It is not reason to break.

breaking happen in build due to CI binding test. you can find the points in this thread.

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

@bzz : I have build PR for data validator. At the same time I will create new PR for leaflet mapping as lot of users are interested.

@randerzander

Copy link
Copy Markdown
Contributor

@Madhuka@bzz what's holding this one up? Is there something someone else can do to get this across the goal line?

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

@randerzander: Thanks for ping!
@bzz : I will create new PR for adding leaflet.

@corneadoug

Copy link
Copy Markdown
Contributor

@randerzander@Madhuka
I'm reviewing #262 first, already took a look at it.
I'm planning on finishing review on those 3 PR this week.
But I think this will need a new PR without data validation and rebased at least.

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

@corneadoug : Thanks, get #262, it contain data validation.
So new PR will have only Map Vis.

@corneadoug

Copy link
Copy Markdown
Contributor

@Madhuka Are you working on a new PR?
Tell me if you need some help

@felixcheung

Copy link
Copy Markdown
Member

any update on this? @Madhuka this would be very cool to have

@ankurmitujjain

Copy link
Copy Markdown

Hi @Madhuka any update on this? really appreciate your effort for this.
Thanks

@corneadoug

Copy link
Copy Markdown
Contributor

After discussion with @Madhuka, this should be rebased and finished by 1st week of March. (since he is busy)

There has been a new PR opened recently (#728), however I'd like Madhuka to finish his contribution for a few reasons:

  • This PR was part of Google Summer of Code, and is all thats left of all the work Madhuka did, so it would be nice for him to get his contribution
  • This PR has been around for quite some time, and I took way too long to properly review it

However if it is not done by then, we should get things moving and go with #728, @eddies fine with that?

@eddies

Copy link
Copy Markdown

@corneadoug absolutely. As I mentioned in #728, I only made my PR out of expediency because I didn't see any movement here. If @Madhuka will be revisiting this PR soon, that would be ideal--he definitely deserves the credit for all his work.

@ankurmitujjain

Copy link
Copy Markdown

@Madhuka Any update on this?

@yanglei99

Copy link
Copy Markdown

When will this be in the main stream?It is a pity to see some great function missing while the solution is already available for more than half a year. Thanks.

@felixcheung

Copy link
Copy Markdown
Member

Based on the earlier comment, are we moving to #728 then?

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

Referee #765

@felixcheung

Copy link
Copy Markdown
Member

Great - will you have time to get that ready then?

_____________________________

@jfuni

Copy link
Copy Markdown

Nice work ! When we will be able to use these nice maps?

@corneadoug

Copy link
Copy Markdown
Contributor

@Madhuka Let's close this one since you opened another one

@Madhuka

Copy link
Copy Markdown
ContributorAuthor

@corneadoug As we have #765 We can close this, I am willing to finish that #765 as lot of people need this feature. Please let me know when I can continue to finish that from your end?

@asfgitasfgit closed this in 3982184Oct 3, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@Madhuka@vgmartinez@eranwitkon@corneadoug@bzz@randerzander@friedhelmvictor@felixcheung@ankurmitujjain@eddies@yanglei99@jfuni@heruka-urgyen