scijava / scijava-jupyter-kernel

[RETIRED] Try IJava or BeakerX
Apache License 2.0
178 stars 42 forks source link

NPE from Clojure maps with nil values #64

Closed kephale closed 7 years ago

kephale commented 7 years ago

In a notebook cell

#!clojure
{['org.renjin/utils "0.8.1906" :scope "runtime"] nil}

leads to:

java.lang.NullPointerException
    at org.scijava.notebook.converter.HTMLNotebookOutputConverter.asHTML(HTMLNotebookOutputConverter.java:42)
    at org.scijava.notebook.converter.MapToHTMLTableNotebookConverter.convert(MapToHTMLTableNotebookConverter.java:71)
    at org.scijava.notebook.converter.MapToHTMLTableNotebookConverter.convert(MapToHTMLTableNotebookConverter.java:39)
    at org.scijava.notebook.converter.NotebookOutputConverter.convert(NotebookOutputConverter.java:50)
    at org.scijava.convert.AbstractConvertService.convert(AbstractConvertService.java:127)
    at org.scijava.jupyter.kernel.evaluator.Worker.run(Worker.java:127)
    at org.scijava.thread.DefaultThreadService$2.run(DefaultThreadService.java:220)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
imagejan commented 7 years ago

That's because the output of the cell is null. I guess the output should simply be ignored by the kernel in this case.

The following doesn't throw an exception:

#!clojure
{['org.renjin/utils "0.8.1906" :scope "runtime"] nil}
5
kephale commented 7 years ago

Actually not quite.

Here is the output from a standard Clojure REPL: `user=> {:a 5}

{:a 5}

user=> {['org.renjin/utils "0.8.1906" :scope "runtime"] nil}

{[org.renjin/utils "0.8.1906" :scope "runtime"] nil}

`

Also note that {:a 5} behaves correctly

imagejan commented 7 years ago

Oh, I see, it's a map conversion issue. The same happen e.g. with groovy:

#!groovy

[a:4, e: null]
java.lang.NullPointerException
    at org.scijava.notebook.converter.HTMLNotebookOutputConverter.asHTML(HTMLNotebookOutputConverter.java:42)
    at org.scijava.notebook.converter.MapToHTMLTableNotebookConverter.convert(MapToHTMLTableNotebookConverter.java:71)
    at org.scijava.notebook.converter.MapToHTMLTableNotebookConverter.convert(MapToHTMLTableNotebookConverter.java:39)
    at org.scijava.notebook.converter.NotebookOutputConverter.convert(NotebookOutputConverter.java:50)
    at org.scijava.convert.AbstractConvertService.convert(AbstractConvertService.java:127)
    at org.scijava.jupyter.kernel.evaluator.Worker.run(Worker.java:127)
    at org.scijava.thread.DefaultThreadService$2.run(DefaultThreadService.java:220)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
hadim commented 7 years ago

This is definitely a conversion issue. @awalter17 any idea?

awalter17 commented 7 years ago

Yeah, the issue is in the MapToHTMLTableNotebookConverter line 71. It should check if table.get(key) != null instead of just checking the key.

I made this change on a branch (fix-map-converter #65), and both the clojure and groovy examples now work. Though the output for the clojure example isn't the prettiest, but I think it is displaying the correct thing (I'm not super familiar with clojure). Please feel free to let me know if there are any issues with my fix.

hadim commented 7 years ago

Awesome. Could you also those code example to notebooks/Test.ipynb ?

awalter17 commented 7 years ago

Sure! I added a clojure and groovy example for the MapToHTMLTableNotebookConverter and a groovy example for ListMapToHTMLTableNotebookConverter.