Skip to content

Create dm_mripicture.py#302

Merged
jerdra merged 8 commits intoTIGRLab:masterfrom
kyjimmy:master
Apr 28, 2021
Merged

Create dm_mripicture.py#302
jerdra merged 8 commits intoTIGRLab:masterfrom
kyjimmy:master

Conversation

@kyjimmy
Copy link
Copy Markdown
Contributor

@kyjimmy kyjimmy commented Apr 26, 2021

Fixed the hard coding issue and changed appearance of the output picture.

kyjimmy added 4 commits March 26, 2021 11:51
This script is designed for TAY cohort study and creates a cropped T1 image for custom t-shirt.
@auto-assign auto-assign bot requested review from edickie, jerdra and josephmje April 26, 2021 16:51
Copy link
Copy Markdown
Contributor

@josephmje josephmje left a comment

Choose a reason for hiding this comment

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

Looking pretty good @kyjimmy! Thanks for this addition. I left a few minor suggestions and some things to think through.

Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Copy link
Copy Markdown
Contributor

@jerdra jerdra left a comment

Choose a reason for hiding this comment

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

Good stuff Jimmy! Just a few additional comments from me

Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py Outdated
Comment thread bin/dm_mripicture.py
Thank you Michael for the comments.
josephmje
josephmje previously approved these changes Apr 28, 2021
Copy link
Copy Markdown
Contributor

@josephmje josephmje left a comment

Choose a reason for hiding this comment

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

Looks good to me @kyjimmy! Also, re the cut_coords, I didn't realize nilearn is using MNI coordinates. It might be fine to keep this hard-coded.

@kyjimmy
Copy link
Copy Markdown
Contributor Author

kyjimmy commented Apr 28, 2021

Looks good to me @kyjimmy! Also, re the cut_coords, I didn't realize nilearn is using MNI coordinates. It might be fine to keep this hard-coded.

Thank you @josephmje. I just realized that I need to update the doc/requirements.txt and add nilearn to the list.

@josephmje
Copy link
Copy Markdown
Contributor

I just realized that I need to update the doc/requirements.txt and add nilearn to the list.

You'll want to update the setup.cfg file instead actually with the nilearn version you're using. the docs/requirements.txt file only contains the minimal python packages required for building the datman documentation.

remove nilearn from requirements.txt and add nilearn to setup.cfg
@jerdra jerdra merged commit 75d6bb2 into TIGRLab:master Apr 28, 2021
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.

3 participants