brainglobe / brainglobe-atlasapi

A lightweight python module to interact with atlases for systems neuroscience
https://brainglobe.info/documentation/brainglobe-atlasapi/index.html
BSD 3-Clause "New" or "Revised" License
124 stars 32 forks source link

Hierarchical structure with treelib #13

Closed FedeClaudi closed 4 years ago

FedeClaudi commented 4 years ago

Heya,

I've used treelib in https://github.com/brainglobe/brainatlas-api/blob/d6adc0e49d844717203576b465f7666b845d742c/brainatlas_api/core.py#L205

and it looks like it's a nice little library that does one thing but it does it very well.

We have a few functions like get_structure_descendants that we could implement using the Allensdk Tree stuff, but I feel like they'd work best with a treelib Tree. I think using the allen stuff to go from structures.json to some hierarchical structure is good as it has a few checks in place to make sure that stuff is correct, but we could build a Tree in parallel and used that for most operations. It's a bit redundant maybe but it offers a very intuitive interface (I'm biased though since I really dislike the way the structure tree is implemented by the AllenSDK).

what do you all think about it?

vigji commented 4 years ago

The package looks quite compact and I would not mind to much adding it. The important thing is that we don't add too much redundancies in the syntaxes or it get soon kind of confusing - I feel that's part of the issue with the Allen one.

Anyway, I moved the StructureTree class from the atlas exactly so that we can change it to be the most convenient for us! This way we can put methods like print_structures_tree in our custom StructureTree class and we can the Atlas class cleaner.

FedeClaudi commented 4 years ago

Anyway, I moved the StructureTree class from the atlas exactly so that we can change it to be the most convenient for us!

That's a great idea! I'll move print_structures and print_structures_tree to it, and will fix one of the things that annoys me the most which is that most methods expect a list and return a list, even if you want to for instance get the id number of a single region.

vigji commented 4 years ago

yup, that's very annoying :D

FedeClaudi commented 4 years ago

Fixed with https://github.com/brainglobe/brainatlas-api/pull/14#issue-424350577