-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
New function to create violin plots from sct_pipeline results #1759
Conversation
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.
Some feedback, to be considered or not
scripts/sct_pipeline_makefig.py
Outdated
def get_parser(): | ||
parser = argparse.ArgumentParser( | ||
description='Generate figure from the output of sct_pipeline.') | ||
parser.add_argument("-i", |
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.
When using argparse for new code, suggest to use short/long arguments:
"-i", "--input",
"-l", "--label",
"-v", "--verbose",
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.
fixed in 9d77c82
scripts/sct_pipeline_makefig.py
Outdated
sct.start_stream_logger() | ||
|
||
# fs = 10 # font size | ||
nb_plots = args.i.__len__() |
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 way, the long argument name will be used as default attribute name (args.input
if above was used).
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.
fixed in 9d77c82
scripts/sct_pipeline_makefig.py
Outdated
list_data = [] | ||
text_results = [] # numerical results to display inside the figure | ||
for fname_pickle in args.i: | ||
df = pickle.load(open(fname_pickle, "rb")) |
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.
Could be changed to io.open()
(more explanatory IMO, but it's a question of taste/habits)
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.
fixed in 9d77c82
scripts/sct_pipeline_makefig.py
Outdated
# | ||
# Generate figure from the output of sct_pipeline. | ||
# | ||
# Copyright (c) 2013 Polytechnique Montreal <www.neuro.polymtl.ca> |
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.
2018
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.
fixed in 9d77c82
|
||
if __name__ == '__main__': | ||
parser = get_parser() | ||
arguments = parser.parse_args() |
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.
Although you can invoke the program with -i file_1 -label A -i file_2 -label B
you will invariably have users who end up doing -i file_1 file_2 -label A B
and end up with -i file_1 file_2 -label A
.
if len(arguments.i) != len(arguments.label):
raise RuntimeError("Mismatch between # of files and labels")
Perhaps the labels should be the file names, if not provided.
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.
fixed in 9d77c82
* New function to create violin plots from sct_pipeline results * Various fixes and improvements Former-commit-id: d40444e
Description of the Change
Implements a new function to generate visual results from
sct_pipeline
.Example usage:
Generated figure:
Future improvements:
sct_pipeline
results and use them in this function (notably to get rid of the flaglabel
)