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

Add args for user-specified metric input file name strings, variables to residualize splits for #15

Merged
merged 3 commits into from
Jan 11, 2022

Conversation

alyssadai
Copy link
Collaborator

Added optional --output_suffix argument in extract_metrics.py (accepts a string) and --residfor argument in define_splits.py (accepts a list).

Users now have more flexibility in naming the single-metric input files (addresses #14), and can run stability analyses where the vertex input in splits are residualized for specified variables from the demographics sheet.

@alyssadai alyssadai changed the title Add arguments for user-specified metric input file name strings, variables to residualize splits for Add args for user-specified metric input file name strings, variables to residualize splits for Jan 5, 2022
@raihaan
Copy link
Collaborator

raihaan commented Jan 10, 2022

Thanks for the changes. In 7da36ae, can you walk me through whats happening in line 147?

resid_vars_a = resid_vars[Asplits_indices[str(split)],:]; resid_vars_b = resid_vars[Bsplits_indices[str(split)],:]

following the code i thought resid_vars is, at this point, a list containing the variables to residualize for, but its being indexed above more like it contains data for each subject? Assuming i'm missing something, lemme know.

@alyssadai alyssadai self-assigned this Jan 10, 2022
@alyssadai alyssadai added the enhancement New feature or request label Jan 10, 2022
@alyssadai alyssadai linked an issue Jan 10, 2022 that may be closed by this pull request
@alyssadai
Copy link
Collaborator Author

Nope, you're right, that's an error on my part. I had the names of residualized variables hard coded in my original copy of the script, and accidentally got rid of the variable storing the corresponding columns from the demographics file when switching to the command-line argument. Should be fixed in 99c9110. Thanks for catching that!

@alyssadai alyssadai requested a review from raihaan January 10, 2022 21:28
@raihaan raihaan merged commit 6696c0c into CoBrALab:argparse Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

more flexible output filenames
2 participants