FZJ-INM1-BDA / siibra-python

Software interfaces for interacting with brain atlases - Python client
Apache License 2.0
48 stars 10 forks source link

[enhancement] map.fetch() returns inconsistent type #396

Open xgui3783 opened 1 year ago

xgui3783 commented 1 year ago

as of 0.4a57

sample code:

import siibra
p = siibra.parcellations["julich brain 2.9"]

type(p.get_map("fsaverage", "labelled").fetch()) # returns <class 'dict'>
type(p.get_map("mni152", "labelled").fetch()) # returns <class 'nibabel.nifti1.Nifti1Image'>

It would be nice to be the returned object to be of the same class, so if the user would like to export all maps to bytes, they could simply call

p.get_map("mni152", "labelled").fetch().to_bytes() # or something similar

edit:

proposal, map should have a to_bytes method, which yields a list of bytes, so users do not need to care the implementation details:

# proposed solution... does not actually work yet

import siibra
p = siibra.parcellations["julich brain 2.9"]

list_bytes = p.get_map("fsaverage", "labelled").to_bytes()
for bytes in list_byptes:
    type(bytes) # bytes
xgui3783 commented 1 year ago

similarly,

import siibra
p = siibra.parcellations["julich brain 2.9"]
p.get_map("big brain", "labelled").fetch()

fails entirely.

AhmetNSimsek commented 1 year ago

similarly,

import siibra
p = siibra.parcellations["julich brain 2.9"]
p.get_map("big brain", "labelled").fetch()

fails entirely.

It throws InsufficientArgumentException, which is the designed and expected output. Do you have an example of default output you'd expect?

AhmetNSimsek commented 1 year ago

as of 0.4a57

sample code:

import siibra
p = siibra.parcellations["julich brain 2.9"]

type(p.get_map("fsaverage", "labelled").fetch()) # returns <class 'dict'>
type(p.get_map("mni152", "labelled").fetch()) # returns <class 'nibabel.nifti1.Nifti1Image'>

It would be nice to be the returned object to be of the same class, so if the user would like to export all maps to bytes, they could simply call

p.get_map("mni152", "labelled").fetch().to_bytes() # or something similar

edit:

proposal, map should have a to_bytes method, which yields a list of bytes, so users do not need to care the implementation details:

# proposed solution... does not actually work yet

import siibra
p = siibra.parcellations["julich brain 2.9"]

list_bytes = p.get_map("fsaverage", "labelled").to_bytes()
for bytes in list_byptes:
    type(bytes) # bytes

I think your proposal makes sense. Later, if we introduce a mesh class instead of a dictionary for surfaces, we can then carry this implementation as well. I'd suggest (if this was not implicit already) is allowing to_bytes to take the same kwargs as fetch.

xgui3783 commented 1 year ago

similarly,

import siibra
p = siibra.parcellations["julich brain 2.9"]
p.get_map("big brain", "labelled").fetch()

fails entirely.

It throws InsufficientArgumentException, which is the designed and expected output. Do you have an example of default output you'd expect?

What if the user just want to export a nifti, to be visualized in their viewer of interest?

The usecase in siibra-api is such that, provided with a parcellation and a template, please return me a nifti/volume file.

AhmetNSimsek commented 9 months ago

By internal meeting in December 2023, this was postponed to be discussed along with the mesh support of siibra-python.

AhmetNSimsek commented 9 months ago

Dev meeting 17.01.2024 notes:

xgui3783 commented 9 months ago

n.b. there is a difference of different fileformat (e.g. gifti, obj, stl etc) and language specific format/library.

For former, there is already a discussion at https://github.com/FZJ-INM1-BDA/siibra-python/issues/13 . For latter, at least for nilearn, they seem to use their own implementation

AhmetNSimsek commented 9 months ago

Dev meeting discussion notes