java.lang.NullPointerException when trying to access AllenUtils.getOntologies().getAncestors()

[judith-baka]

Describe the bug
You get java.lang.NullPointerException when trying to access AllenUtils.getOntologies().getAncestors()

To Reproduce

from scyjava import jimport
AllenUtils = jimport('sc.fiji.snt.annotation.AllenUtils')
areas = AllenUtils.getOntologies()
for area in areas:
     ancestors = area.getAncestors()

gives "java.lang.NullPointerException"

But there is a hint:
if you do:

from scyjava import jimport
AllenUtils = jimport('sc.fiji.snt.annotation.AllenUtils')
areas = AllenUtils.getOntologies()
for area in areas:
     **parent = area.getAncestor(1)**
     ancestors = area.getAncestors()

no error..

[carshadi]

Hi @judith-baka , thanks for catching this. It is indeed a bug. What is happening is that getAncestors() attempts to call subList() on the parentStructure attribute of the AllenCompartment instance before this attribute is initialized, thus the NPE.

Internally, this initialization is meant to be done on an "as-needed" basis (i.e., on the first call of an AllenCompartment method that actually uses the parentStructure ArrayList), by calling getTreePath(), which populates parentStructure once and then returns the list on subsequent calls. However, this step is currently missing from getAncestors(), as you have discovered. So, an immediate workaround could be something like:

from scyjava import jimport
AllenUtils = jimport('sc.fiji.snt.annotation.AllenUtils')
areas = AllenUtils.getOntologies()
for a in areas:
    tree_path = list(ij.py.from_java(a.getTreePath()))
    ancestors = tree_path[:-1]
    if not ancestors:
        print(tree_path[0].name()) # Whole Brain

On my machine, ij.py.from_java(a.getTreePath()) returns a <class 'scyjava.JavaList'> instead of a Python list, which warranted the cast, though I'm not sure if that is a bug with pyimagej (I'm running version 1.0.0)

The reason for not initializing parentStructure within the AllenCompartment constructor is that it would trigger a chain reaction of unnecessary object creation, since getTreePath() constructs a new AllenCompartment for each identifier in the tree path.

So, one easy fix that immediately comes to mind is to replace all direct manipulation of the parentStructure list with a call to getTreePath(). @tferr what do you think? Any alternatives?

[tferr]

calling getTreePath() should work.

On my machine, ij.py.from_java(a.getTreePath()) returns a <class 'scyjava.JavaList'> instead of a Python list, which warranted the cast, though I'm not sure if that is a bug with pyimagej (I'm running version 1.0.0)

I noticed that too. Maybe we are being too lax? Do collections exist in Python? Maybe the cast is not needed if we return a List rather than a Collection?:

	public static List<AllenCompartment> getOntologies() {
		return new AllenTreeModel().getOntologies(); // Change this to also return a List
	}

Either way, it is probably the right thing to do? I had though of always using the most general representation, bu Collection may be an exaggeration? Specially if we want to API to be script-friendly?

[carshadi]

agreed

[tferr]

If adopting list works, we probably should extend it to other classes too

[carshadi]

I'll start investigating 🕵️‍♂️

[carshadi]

@tferr the PR I just linked has fixes for most of what we talked about here.

RE: the migration of public methods returning Collection, I updated AllenUtils.getOntologies() in this PR. There are 10 or so other public API methods outside of the Allen Classes that return Collection.

Would it make sense to open a different issue for those?

[tferr]

Would it make sense to open a different issue for those?

Yes. Definitely. Does it solve the pyimagej issue?

[carshadi]

from scyjava import jimport
AllenUtils = jimport('sc.fiji.snt.annotation.AllenUtils')
areas = AllenUtils.getOntologies()
print(type(areas))  # <java class 'java.util.ArrayList'>
print(type(ij.py.from_java(areas)))  # <class 'scyjava.JavaList'>

Apparently not...
ArrayList is supported according to https://github.com/imagej/pyimagej/blob/master/doc/Usage.md#converting-between-java-and-python

[tferr]

bummer. Maybe we should open an issue in pyimagej? Now that we are returning lists, seems rather silly that it is not casted immediately?

[tferr]

Oh, wait, didn't see the link (was reading on the phone). Shall we return ArrayList instead?
Can you please ask the pyimagej team if there is any chance for Lists to be supported in the future?

[carshadi]

I'll try that, sure

[carshadi]

I get the same result when returning ArrayList

[tferr]

bummer. let's open the issue. Meanwhile I'll try to release a new version over the weekend. I am looking at the double entry in linux.

[carshadi]

sounds good 👍

[carshadi]

@tferr in case you didn't see this imagej/pyimagej#100
So it seems we could just cast with list() and omit ij.py.from_java() to get a true Python list

[tferr]

nice. way more concise. Do you have time to update the notebooks?

[carshadi]

on it