nilearn icon indicating copy to clipboard operation
nilearn copied to clipboard

Ensure `fetch_atlas_*` functions returns one atlas map only

Open htwangtw opened this issue 2 years ago • 5 comments

Some of the possibly older multiscale atlas fetcher is slightly inconsistent with how the later implemented ones For example, nilearn.dataset.fetch_atlas_basc_multiscale_2015 would return all resolutions: https://github.com/nilearn/nilearn/blob/2fd6665664f27dfbf1f10aeeba660adc9b560361/nilearn/datasets/atlas.py#L965-L972

Craddock returns all atlas construction methods https://github.com/nilearn/nilearn/blob/1607b52458c28953a87bbe6f42448b7b4e30a72f/nilearn/datasets/atlas.py#L142-L148

For some latter implemented ones - Difumo would require resolution to be specified: https://github.com/nilearn/nilearn/blob/1607b52458c28953a87bbe6f42448b7b4e30a72f/nilearn/datasets/atlas.py#L46-L58

Julich needs atlas name: https://github.com/nilearn/nilearn/blob/1607b52458c28953a87bbe6f42448b7b4e30a72f/nilearn/datasets/atlas.py#L363-L370

Benefits to the change

Improve consistency. Ensuring fetch_atlas_* functions only return one atlas map would be more intuitive. I hope the atlas path will be under the label maps, so it's consistent amongst atlas fetchers.

Pseudocode for the new behavior, if applicable

Using nilearn.dataset.fetch_atlas_basc_multiscale_2015 as an example:

>> parcellations = fetch_atlas_basc_multiscale_2015(resolution=None)  # old behaviour
>> parcellations.keys()
dict_keys(['scale007', 'scale012', 'scale020', 'scale036', 'scale064', 'scale122', 'scale197', 'scale325', 'scale444', 'description'])
>>parcellations.scale007
'/path/to/nilearn_data/basc_multiscale_2015/template_cambridge_basc_multiscale_nii_sym/template_cambridge_basc_multiscale_sym_scale007.nii.gz'


>> parcellation = fetch_atlas_basc_multiscale_2015(resolution=7) 
>> parcellation.keys()
dict_keys(['maps', 'description'])
>> parcellations.map
'/path/to/nilearn_data/basc_multiscale_2015/template_cambridge_basc_multiscale_nii_sym/template_cambridge_basc_multiscale_sym_scale007.nii.gz'

Are there any other older atlas fetcher that I am missing here? Happy to do the PR!

htwangtw avatar Nov 23 '21 20:11 htwangtw

I think it makes sense as it would increase consistency across fetchers. Happy to review the PR! :)

NicolasGensollen avatar Nov 24 '21 06:11 NicolasGensollen

Indeed ! A related point is that the terminology is a bit noisy, where we alternatively talk about 'scale', 'dimension', 'resolution'. Having a more unified terminology would help too. Thx !

bthirion avatar Nov 24 '21 07:11 bthirion

Indeed ! A related point is that the terminology is a bit noisy, where we alternatively talk about 'scale', 'dimension', 'resolution'. Having a more unified terminology would help too. Thx !

Good point, especially given the contribution guideline now has some explicit requirements for atlas fetchers: https://nilearn.github.io/stable/development.html#how-to-contribute-an-atlas

htwangtw avatar Nov 24 '21 15:11 htwangtw

@bthirion I can work on this

achamma723 avatar Aug 23 '22 14:08 achamma723

Thx @achamma723, that would be great!

ymzayek avatar Aug 23 '22 17:08 ymzayek