Skip to content

[ZEPPELIN-127] Fix issue raised by ZEPPELIN-127 related to DisplayUtils#133

Closed
doanduyhai wants to merge 2 commits intoapache:masterfrom
doanduyhai:FixDisplayUtils
Closed

[ZEPPELIN-127] Fix issue raised by ZEPPELIN-127 related to DisplayUtils#133
doanduyhai wants to merge 2 commits intoapache:masterfrom
doanduyhai:FixDisplayUtils

Conversation

@doanduyhai
Copy link
Copy Markdown
Contributor

@doanduyhai doanduyhai commented Jun 30, 2015 •
edited
Loading

Dean1.Wood ([email protected]) reported the problem. Following code is throwing exception.

val wbIPR=Array(8.40,      8.09,      7.56,      6.83,      5.88,      4.72,      3.36,      1.78,      0.0)
val pressureIPR=Array(0.00,10000000.00,20000000.00,30000000.00,40000000.00,50000000.00,60000000.00,70000000.00,80000000.00)
case class IPR (w:Array[Double],p:Array[Double])
val iprInit=IPR(wbIPR,pressureIPR)
val iprList=sc.parallelize((1.0 to 0.9 by -0.001).map(k => IPR(iprInit.w.map(_ * k), iprInit.p)).zipWithIndex,5)
iprList.count()

To fix the issue, simple trick: wrap the implicit value around an object and import it

intp.interpret("object MaxResult {" +
        "implicit val sparkMaxResult = new SparkMaxResult(" +
        Integer.parseInt(getProperty("zeppelin.spark.maxResult")) + ")" +
        "}");
intp.interpret("import MaxResult.sparkMaxResult");

@doanduyhai doanduyhai changed the title Fix issue raised by ZEPPELIN-127 related to DisplayUtils [ZEPPELIN-127] Fix issue raised by ZEPPELIN-127 related to DisplayUtils Jul 1, 2015
@Leemoonsoo
Copy link
Copy Markdown
Member

Leemoonsoo commented Jul 1, 2015

Thanks for submitting the fix. Let me try it.

@Leemoonsoo
Copy link
Copy Markdown
Member

Leemoonsoo commented Jul 2, 2015

Tested with following case and Looks good to me.

val wbIPR=Array(8.40,      8.09,      7.56,      6.83,      5.88,      4.72,      3.36,      1.78,      0.0)
val pressureIPR=Array(0.00,10000000.00,20000000.00,30000000.00,40000000.00,50000000.00,60000000.00,70000000.00,80000000.00)
case class IPR (w:Array[Double],p:Array[Double])
val iprInit=IPR(wbIPR,pressureIPR)
val iprList=sc.parallelize((1.0 to 0.9 by -0.001).map(k => IPR(iprInit.w.map(_ * k), iprInit.p)).zipWithIndex,5)
iprList.count()

@doanduyhai
Copy link
Copy Markdown
Contributor Author

doanduyhai commented Jul 15, 2015

Would this PR make it for 0.5.0 or delayed to 0.6.x ?

@Leemoonsoo
Copy link
Copy Markdown
Member

Leemoonsoo commented Jul 15, 2015

I think it's better to have this PR on 0.6.x, because of this PR enabling new feature while 0.5.x are finalized and in a voting process.
Once it's on master branch, then I think we can have discussion about backporting it to 0.5.1 if we really want to this feature on 0.5.x branch.

@doanduyhai
Copy link
Copy Markdown
Contributor Author

doanduyhai commented Jul 15, 2015

Ok, sounds reasonable. Let's release 0.5.0 with stable features then

@doanduyhai doanduyhai force-pushed the FixDisplayUtils branch 2 times, most recently from 82c16d3 to 0f88e2d Compare July 31, 2015 16:02
@corneadoug
Copy link
Copy Markdown
Contributor

corneadoug commented Sep 27, 2016

@doanduyhai Do we have the still the need of that PR?

@doanduyhai
Copy link
Copy Markdown
Contributor Author

doanduyhai commented Sep 27, 2016

@corneadoug Yes we need it, it's a very convenient helper class to display html/img ...

@corneadoug
Copy link
Copy Markdown
Contributor

corneadoug commented Sep 28, 2016

@doanduyhai Then can you create a Jira issue for it and maybe update the PR description?

@doanduyhai
Copy link
Copy Markdown
Contributor Author

doanduyhai commented Oct 3, 2016

@corneadoug Comment updated with original issue description.

The JIRA is already created: https://issues.apache.org/jira/browse/ZEPPELIN-127

@corneadoug
Copy link
Copy Markdown
Contributor

corneadoug commented Oct 4, 2016

@doanduyhai Thanks, since it wasn't in the description, I didn't know
@Leemoonsoo Can I merge this?

@Leemoonsoo
Copy link
Copy Markdown
Member

Leemoonsoo commented Oct 5, 2016

master branch runs

val wbIPR=Array(8.40,      8.09,      7.56,      6.83,      5.88,      4.72,      3.36,      1.78,      0.0)
val pressureIPR=Array(0.00,10000000.00,20000000.00,30000000.00,40000000.00,50000000.00,60000000.00,70000000.00,80000000.00)
case class IPR (w:Array[Double],p:Array[Double])
val iprInit=IPR(wbIPR,pressureIPR)
val iprList=sc.parallelize((1.0 to 0.9 by -0.001).map(k => IPR(iprInit.w.map(_ * k), iprInit.p)).zipWithIndex,5)
iprList.count()

with out this patch. Do we still need this patch?

@doanduyhai
Copy link
Copy Markdown
Contributor Author

doanduyhai commented Oct 5, 2016

@Leemoonsoo

Yes we do need this patch because DisplayUtils has been disabled temporarily because of the issue it causes. This patch fixes the issue and re-enable DisplayUtils

image

@Leemoonsoo
Copy link
Copy Markdown
Member

Leemoonsoo commented Oct 5, 2016

@doanduyhai I see. Thanks for the explanation.

Current master branch works with both scala-2.10 and scala-2.11 without rebuild, depending on which version of spark SPARK_HOME points. is displayUtils works in this way, too?

@doanduyhai
Copy link
Copy Markdown
Contributor Author

doanduyhai commented Oct 9, 2016

@Leemoonsoo

The fix is using pretty standard Scala (object xxx extends Serializable) and not impacted by Scala 2.11 breaking changes

@asfgit asfgit closed this in c38a0a0 May 9, 2018
asfgit pushed a commit that referenced this pull request May 9, 2018
@jongyoul
Close outdated PRs
b11d355 
close #83
close #86
close #125
close #133
close #139
close #146
close #193
close #203
close #246
close #262
close #264
close #273
close #291
close #299
close #320
close #347
close #389
close #413
close #423
close #543
close #560
close #658
close #670
close #728
close #765
close #777
close #782
close #783
close #812
close #822
close #841
close #843
close #878
close #884
close #918
close #989
close #1076
close #1135
close #1187
close #1231
close #1304
close #1316
close #1361
close #1385
close #1390
close #1414
close #1422
close #1425
close #1447
close #1458
close #1466
close #1485
close #1492
close #1495
close #1497
close #1536
close #1545
close #1561
close #1577
close #1600
close #1603
close #1678
close #1695
close #1739
close #1748
close #1765
close #1767
close #1776
close #1783
close #1799
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@doanduyhai @Leemoonsoo @corneadoug