-
Notifications
You must be signed in to change notification settings - Fork 1
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
adding code and readme #1
base: main
Are you sure you want to change the base?
Conversation
Using local .gitignore instead for personal files
…pulation file and mriqc tsv creating file)
…ioral analysis of the nback task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a bunch of comments in various places. the main one i would say that you should look into is the replacement of missing with 0.
you may not want to delete the .gitignore
as it helps not add things like DS store into the git commit.
" group_design_matrix['total_cudit'] = df_subs['subs'].map(dict_MM_baseline_cudit)\n", | ||
" group_design_matrix['total_cudit'] = group_design_matrix['total_cudit'] - group_design_matrix['total_cudit'].mean()\n", | ||
" #needed because MM_141 is nan, so replacing with 0, which is the mean\n", | ||
" group_design_matrix['total_cudit'].fillna(0, inplace=True)\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing issue. check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was doing imputation by the mean -- it's not the best but it's not a major part of the analysis and i thought it would be ok. what do you suggest instead? you think i should remove the subject?
(same comment as for other area where this happened)
" stat_map = contrast_output['z_score']\n", | ||
" effect_map = contrast_output['effect_size']\n", | ||
"\n", | ||
" #for \"positive\" activation difference (nicer visualization)\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is used, make sure that the figure description says so. why not just use the reverse contrast definition in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was a quick fix to not have to rerun things. do you think it's worth editing it all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aren't group contrasts easy to compute? you could flip the contrast at the group level, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is more a notion of the kind of thing that gets lost in details. I don't think you need to change the code at the moment, but make sure that in the paper wherever this contrast is mentioned or shown, the appropriate contrast is described.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also make a note in the place where the contrast is specified in the script that the plots reverse the effect.
restore previous version that was accidentally messed up when replying to a comment in my pull request
add explanation for why certain subs are excluded from the cannabis table
adding specifics to analysis instructions
changes to readme to update necessary coding environment and to include detailed steps to carry out preprocessing
adding -fwhm flag to cifti-smoothing hcp workbench command
add error catch for incorrect number of axes in cifti header
adding note about CUD at baseline MMC subject also in function that removes them
remove additional unneeded line about subs to be excluded based on CUD at baseline
add note to function where subs are removed based on CUD at baseline why they are removed
rename function call avoiding the word "final"
avoid using the word final in function name
Adding code necessary to run preprocessing and analysis for the paper in its current state. Adding README to describe how the project directory should be set up to recreate this project and how the code should be executed.