Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RF + DOC: Add MNI template reference. Also import it into the dipy.da… #681

Merged
merged 4 commits into from Aug 6, 2015

Conversation

arokem
Copy link
Contributor

@arokem arokem commented Jul 16, 2015

…ta namespace.

@samuelstjean
Copy link
Contributor

It would also be a good idea to add the template license in the note (provided thsi is the right one of course).

And the website : http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152NLin2009

License
Copyright (C) 1993–2004 Louis Collins, McConnell Brain Imaging Centre, Montreal Neurological Institute, McGill University. Permission to use, copy, modify, and distribute this software and its documentation for any purpose and without fee is hereby granted, provided that the above copyright notice appear in all copies. The authors and McGill University make no representations about the suitability of this software for any purpose. It is provided “as is” without express or implied warranty. The authors are not responsible for any data loss, equipment damage, property loss, or injury to subjects or patients resulting from the use or misuse of this software package.

@arokem
Copy link
Contributor Author

arokem commented Jul 18, 2015

Sure. Added.

As required by this license, the license also gets downloaded together with the files and saved in the same folder as the template files.

@samuelstjean
Copy link
Contributor

Ah I did not know that. Probably better to put it here also, I doubt that many people will go and read it inside a hidden download folder anyway.

@arokem
Copy link
Contributor Author

arokem commented Jul 18, 2015

Yeah - we don't pay for extra lines of docstrings :-)

And it's better to say these things more times than absolutely necessary,
than too few.

On Sat, Jul 18, 2015 at 10:34 AM, Samuel St-Jean notifications@github.com
wrote:

Ah I did not know that. Probably better to put it here also, I doubt that
many people will go and read it inside a hidden download folder anyway.


Reply to this email directly or view it on GitHub
#681 (comment).

@arokem
Copy link
Contributor Author

arokem commented Aug 5, 2015

Just rebased this one, so it can be cleanly merged.

"""
folder = pjoin(dipy_home, 'mni_template')
files, folder = fetch_mni_template()
file_dict = {"T1":pjoin(folder, 'mni_icbm152_t1_tal_nlin_asym_09a.nii'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @arokem, why do you prefer the ICBM152 2009a version and not the 2009c version?
http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152NLin2009

Also now that you are on it can you also fetch the rest of the maps PDs etc?
We can definitely use them for other projects like the Tissue classifiers etc.

I would recommend fetching the 1x1x1 atlases but also fetching the 0.5x0.5x0.5
atlases those will be useful for performing high resolution anatomical tracking.

I hope this is not much to ask. It would be great to fetch the IIT atlas too at some point.
I will need to ask permission from Arfanakis for that. But I think it's possible!
Exciting!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'c' atlas is symmetric, and so does not represent an average brain. It's main use is when you want to compare directly between the two hemispheres, so I don't think it should be the default.

It sounds like the IIT atlas has an inconvenient license, so we should be careful when fetching that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt about that @matthew-brett It clearly says in the MNI website
http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152NLin2009
that there is a symmetric and asymmetric version of the 'c' atlas.
Look at the last bullet point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know well the person who wrote the atlas and I don't think he had the intention to have a hard-boiled license. I will speak to him about that. Maybe they would love to update their license. Or they don't mind fetching it in DIPY.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes - sorry - you are right about 2009c. Then - do you know the difference between 2009a and 2009c? The page mentions only the different version numbers for the intensity correction method and different 'resampling'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No rush. The ROIs that we are using for tractography segmentation are all
based on the MNI template anyway.

On Thu, Aug 6, 2015 at 10:18 AM, Samuel St-Jean notifications@github.com
wrote:

In dipy/data/fetcher.py
#681 (comment):

 """
  • folder = pjoin(dipy_home, 'mni_template')
  • files, folder = fetch_mni_template()
    file_dict = {"T1":pjoin(folder, 'mni_icbm152_t1_tal_nlin_asym_09a.nii'),

They also have a seperate license for commercial and non commercial usage,
so maybe you can just point a download link for people while the
re-licensing happens, so that people might have it in the meantime if it's
that important.


Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/pull/681/files#r36440520.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(that might also be the reason we are using 'a')

On Thu, Aug 6, 2015 at 10:20 AM, Ariel Rokem arokem@gmail.com wrote:

No rush. The ROIs that we are using for tractography segmentation are all
based on the MNI template anyway.

On Thu, Aug 6, 2015 at 10:18 AM, Samuel St-Jean notifications@github.com
wrote:

In dipy/data/fetcher.py
#681 (comment):

 """
  • folder = pjoin(dipy_home, 'mni_template')
  • files, folder = fetch_mni_template()
    file_dict = {"T1":pjoin(folder, 'mni_icbm152_t1_tal_nlin_asym_09a.nii'),

They also have a seperate license for commercial and non commercial
usage, so maybe you can just point a download link for people while the
re-licensing happens, so that people might have it in the meantime if it's
that important.


Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/pull/681/files#r36440520.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no preference for a as oppose to c. If c is better, let's go with
that

On Thursday, August 6, 2015, Ariel Rokem notifications@github.com wrote:

In dipy/data/fetcher.py
#681 (comment):

 """
  • folder = pjoin(dipy_home, 'mni_template')
  • files, folder = fetch_mni_template()
    file_dict = {"T1":pjoin(folder, 'mni_icbm152_t1_tal_nlin_asym_09a.nii'),

No rush. The ROIs that we are using for tractography segmentation are all
based on the MNI template anyway.
… <#14f040978a952b82_>
On Thu, Aug 6, 2015 at 10:18 AM, Samuel St-Jean <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote: In
dipy/data/fetcher.py <
https://github.com/nipy/dipy/pull/681#discussion_r36440520>: > """ > -
folder = pjoin(dipy_home, 'mni_template') > + files, folder =
fetch_mni_template() > file_dict = {"T1":pjoin(folder,
'mni_icbm152_t1_tal_nlin_asym_09a.nii'), They also have a seperate license
for commercial and non commercial usage, so maybe you can just point a
download link for people while the re-licensing happens, so that people
might have it in the meantime if it's that important. — Reply to this email
directly or view it on GitHub <
https://github.com/nipy/dipy/pull/681/files#r36440520>.


Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/pull/681/files#r36440755.

Jason D. Yeatman, PhD
Assistant Professor, Institute for Learning & Brain Sciences (I-LABS)
Department of Speech & Hearing Sciences
University of Washington
http://depts.washington.edu/bdelab/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably the ROIs were made based on 'a'?

On Thu, Aug 6, 2015 at 11:03 AM, Jason D. Yeatman notifications@github.com
wrote:

In dipy/data/fetcher.py
#681 (comment):

 """
  • folder = pjoin(dipy_home, 'mni_template')
  • files, folder = fetch_mni_template()
    file_dict = {"T1":pjoin(folder, 'mni_icbm152_t1_tal_nlin_asym_09a.nii'),

I have no preference for a as oppose to c. If c is better, let's go with
that
… <#14f042e76c609d20_>
On Thursday, August 6, 2015, Ariel Rokem notifications@github.com
wrote: In dipy/data/fetcher.py <
https://github.com/nipy/dipy/pull/681#discussion_r36440755>: > """ > -
folder = pjoin(dipy_home, 'mni_template') > + files, folder =
fetch_mni_template() > file_dict = {"T1":pjoin(folder,
'mni_icbm152_t1_tal_nlin_asym_09a.nii'), No rush. The ROIs that we are
using for tractography segmentation are all based on the MNI template
anyway. … <#14f040978a952b82_> On Thu, Aug 6, 2015 at 10:18 AM, Samuel
St-Jean <notifications@github.com <javascript:_e(%7B%7D,'cvml','
notifications@github.com');>> wrote: In dipy/data/fetcher.py <
https://github.com/nipy/dipy/pull/681#discussion_r36440520>: > """ > -
folder = pjoin(dipy_home, 'mni_template') > + files, folder =
fetch_mni_template() > file_dict = {"T1":pjoin(folder,
'mni_icbm152_t1_tal_nlin_asym_09a.nii'), They also have a seperate license
for commercial and non commercial usage, so maybe you can just point a
download link for people while the re-licensing happens, so that people
might have it in the meantime if it's that important. — Reply to this email
directly or view it on GitHub <
https://github.com/nipy/dipy/pull/681/files#r36440520>. — Reply to this
email directly or view it on GitHub <
https://github.com/nipy/dipy/pull/681/files#r36440755>.
-- Jason D. Yeatman, PhD Assistant Professor, Institute for Learning &
Brain Sciences (I-LABS) Department of Speech & Hearing Sciences University
of Washington http://depts.washington.edu/bdelab/


Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/pull/681/files#r36445341.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guys, you can fetch both and check if the ROIs correspond.

@arokem
Copy link
Contributor Author

arokem commented Aug 6, 2015

Also, this is just a refactor of a previous PR. So, we can do all that other stuff, but on another PR.

@Garyfallidis
Copy link
Contributor

Sure, no problem.

@Garyfallidis
Copy link
Contributor

Let's go with 'a' for now and we write another PR for 'c' and the other maps.

Garyfallidis added a commit that referenced this pull request Aug 6, 2015
RF + DOC: Add MNI template reference. Also import it into the dipy.da…
@Garyfallidis Garyfallidis merged commit 93ce2ed into dipy:master Aug 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants