Conversation
MarcCote
force-pushed
the
add_python3_support
branch
from
12cf403 to
23fc412
Compare
|
This makes #89 Invalid. |
scripts/smart_dispatch.py
Outdated
| parser.add_argument('-g', '--gpusPerCommand', type=int, required=False, help='How many gpus a command needs.', default=1) | ||
| # parser.add_argument('-m', '--memPerCommand', type=float, required=False, help='How much memory a command needs (in Gb).') | ||
| parser.add_argument('-f', '--commandsFile', type=file, required=False, help='File containing commands to launch. Each command must be on a seperate line. (Replaces commandAndOptions)') | ||
| parser.add_argument('-f', '--commandsFile', type=str, required=False, help='File containing commands to launch. Each command must be on a seperate line. (Replaces commandAndOptions)') |
|
Not familiar with the code base, but with the comments addressed, the changes look fine to me. |
| @@ -1,7 +1,7 @@ | |||
| language: python | |||
There was a problem hiding this comment.
Wh should add unit test in python 3
There was a problem hiding this comment.
Already done :-), check below.
MarcCote
force-pushed
the
add_python3_support
branch
from
59c4d0c to
cc98728
Compare
|
Thank you @ASalvail for your comments. |
|
@mgermain I added a unit test for the --commandsFile option. Make sure you check https://github.com/SMART-Lab/smartdispatch/pull/106/files#diff-f3e1285c65572b36c9b0f21b0734c260 |
| end = int(groups[1]) | ||
| step = 1 if groups[2] is None else int(groups[2]) | ||
| return map(str, range(start, end, step)) | ||
| return [str(i) for i in range(start, end, step)] |
There was a problem hiding this comment.
Were we really using map everywhere for no reasons ?
There was a problem hiding this comment.
Yes and no. In Python 3, map returns a generator instead of a list. What we want here is a list, so we are better off using list comprehension instead.
MarcCote
force-pushed
the
add_python3_support
branch
from
cc98728 to
c85fb5f
Compare
…ytes to str and removed function file in argparse
MarcCote
force-pushed
the
add_python3_support
branch
from
c85fb5f to
6eb7f8e
Compare
|
See PR #161 |
4 participants
It's all in the title. :D
Maybe @ASalvail could you have a quick look at this one whenever you have time.
Edit: closes #89