search

BlueBrain / voxcell

Tools to work with voxel based brain atlases.
Apache License 2.0
5 stars 3 forks source link

Allow to provide the filename of the brain annotation volume #34

Open lecriste opened 3 months ago

lecriste commented 3 months ago

voxcell/nexus/voxelbrain.py contains a hardcoded filename for the annotation volume ('brain_regions').

mgeplf commented 2 months ago

For the record, I'm not a big fan of changing this behavior.

From an historical point, this atlas layout has been around for a long time, it's mentioned in 2018 here: https://bbpteam.epfl.ch/project/spaces/display/NEOCORTEX/Atlas+Building+and+Validation#AtlasBuildingandValidation-regiondata and existed in the voxels service that the precursor to DKE/NISE described here: https://bbpteam.epfl.ch/project/spaces/display/NRINF/Voxel+Brain+REST+API

In addition, other tools depend on this behavior, as the atlases are laid out in specific manor, so that client code doesn't need to worry about these differences. By not adhering to the standard that was agreed upon before, it increases the friction of others who depend on the library. This usually means that because the files aren't named to the standard get renamed, or the symlinks are created instead.

Finally, the grouping of brain_regions.nrrd and hierarchy.json in the same directory nicely group them, sort of like a namespace, so that it's clear that they are related - which is something that is very important. If the wrong hierarchy.json file is used with a brain_region.nrrd, one will have incorrect results. Storing them in a directory, and naming them consistently reduces the chances that this sort of mistake will happen.

I have also not really seen a good justification on why to move away from this standard, which is why I'm reluctant to change it.

lecriste commented 2 months ago

My idea is to introduce just an optional argument for the annotation volume filename, which defaults to the current brain_regions, so the standard behaviour will not change.

mgeplf commented 2 months ago

I understand that, however, I'm trying to take a step back and look at the bigger picture. The functions within this files are specifically for dealing with VoxelBrain data, which has a certain layout, as I've noted above. It is built for convenience when working with data of this type, and adding flexibility incentivizes alternate layouts, when makes other peoeple's usage of the data harder and it turns into a vicious cycle: what was once convenient becomes inconvenient as more options proliferate.

lecriste commented 2 months ago

What is the inconvenience of having an optional argument? I don't see how it makes other people's usage of the data harder.

mgeplf commented 2 months ago

The inconvenience is that if it exists, then that means people will start laying out atlases differently, and then other people will start having to add argument. By being rigorous and standardized, there is no burden.