Issue #29

[ntraut]

Well, in fact it has nothing to do with the issue of decomposing the steps, but I personally need this because I am unable to run docker in my cluster (nor singularity). All that it costs is the necessity to install python 2 in addition to python 3, but the gain is to be compatible with the original FreeSurfer.

Le 16 mai 2017 à 18:55, Chris Filo Gorgolewski notifications@github.com a écrit :

@chrisfilo commented on this pull request.

In run.py:

@@ -430,7 +454,7 @@ def run(command, env={}, ignore_errors=False): if os.path.isfile(table_file): warn("Replace old file %s" % table_file) os.remove(table_file)

• cmd = "python3 which aparcstats2table --hemi {h} --subjects {subjects} --parc {p} --meas {m} " \
• cmd = "python2 which aparcstats2table --hemi {h} --subjects {subjects} --parc {p} --meas {m} " \ Why is this switch necessary?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[chrisgorgo]

Do you mind splitting those two set of modifications into two PRs? It will make it easier to review and will almost certainly lead to faster merge for one of them.

On Tue, May 16, 2017 at 10:43 AM, Nicolas Traut notifications@github.com wrote:

Well, in fact it has nothing to do with the issue of decomposing the steps, but I personally need this because I am unable to run docker in my cluster (nor singularity). All that it costs is the necessity to install python 2 in addition to python 3, but the gain is to be compatible with the original FreeSurfer.

Le 16 mai 2017 à 18:55, Chris Filo Gorgolewski notifications@github.com a écrit :

@chrisfilo commented on this pull request.

In run.py:

@@ -430,7 +454,7 @@ def run(command, env={}, ignore_errors=False): if os.path.isfile(table_file): warn("Replace old file %s" % table_file) os.remove(table_file)

• cmd = "python3 which aparcstats2table --hemi {h} --subjects {subjects} --parc {p} --meas {m} " \
• cmd = "python2 which aparcstats2table --hemi {h} --subjects {subjects} --parc {p} --meas {m} " \ Why is this switch necessary?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BIDS-Apps/freesurfer/pull/30#issuecomment-301859462, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOkp5EuX3C-owthMhB8rmD9EDiiyAVuks5r6eA6gaJpZM4Ncx-2 .

[ntraut]

I see there is a problem with the path of the file version. Is there a reason to put it in a distinct folder than run.py? I see that in the example app, both are in the root folder...

[chrisgorgo]

There is no reason and it could be moved, but the Dockerfile and circle.yml would have to be adjusted. This also seems like a material for a separate PR ;)

[ntraut]

But I don't see any line which need to be modified in circle.yml... And sorry for that useless question but which one to move: file version in folder /code or file run.py in folder /?

[chrisgorgo]

Actually I don't think you need to change anything in circle. Moving version or run.py should work equally well - don't have preference.

[chrisgorgo]

This looks good to me, but there are a lot of changes. @fliem could you also have a look?

[fliem]

ahhh. I think this is ready to merge, is it? If yes we can also close #23