Allow lower resolution T2s

[Shotgunosine]

This PR address #51. It's not pretty, but I think it should work.

[Shotgunosine]

@PeerHerholz Any idea why this build is failing now? I don't really understand what changed.

[PeerHerholz]

@Shotgunosine thx for working on this! I'm also not entirely sure, but based on the error:

Step 9/23 : RUN bash -c 'pip3 install nibabel pandas==0.21.0'

 ---> Running in 91c4927c5c9f

Collecting nibabel

  Downloading https://files.pythonhosted.org/packages/0e/50/d1f4cc5ea96649af28238715a8ffd05933f56a0f51467d5e3bf84ee4cd27/nibabel-3.1.0-py3-none-any.whl (3.3MB)

Collecting pandas==0.21.0

  Downloading https://files.pythonhosted.org/packages/85/05/e05bb5b117aae8a1cd5d8944db14d5706057aa45415d41f4749fa456182c/pandas-0.21.0-cp35-cp35m-manylinux1_x86_64.whl (25.7MB)

Collecting packaging>=14.3 (from nibabel)

  Downloading https://files.pythonhosted.org/packages/46/19/c5ab91b1b05cfe63cccd5cfc971db9214c6dd6ced54e33c30d5af1d2bc43/packaging-20.4-py2.py3-none-any.whl

Collecting numpy>=1.13 (from nibabel)

  Downloading https://files.pythonhosted.org/packages/f1/2c/717bdd12404c73ec0c8c734c81a0bad7048866bc36a88a1b69fd52b01c07/numpy-1.19.0.zip (7.3MB)

    Complete output from command python setup.py egg_info:

    Traceback (most recent call last):

      File "<string>", line 1, in <module>

      File "/tmp/pip-build-1djlzr5i/numpy/setup.py", line 30, in <module>

        raise RuntimeError("Python version >= 3.6 required.")

    RuntimeError: Python version >= 3.6 required.

it appears that setuptools might have changed something regarding pip installs. Based on that I went through the Dockerfile and we have multiple pips (here). While I think it's unrelated to the error, we should change that, might be a leftover from the python 2 install. Regarding the actual problem, it might be worth trying to specify python 3.6 during the install (here). WDYT?

[Shotgunosine]

@PeerHerholz This is passing tests now and is ready for you to take a look. Shifted python package management to Conda.

[PeerHerholz]

Hi @Shotgunosine,

great, thanks a lot. Looks good, eh? I followed the PRs a bit, but still would like to ask if you could maybe briefly outline the encountered problems? I'll of course merge it, it's just that I know and don't mix things up!

[Shotgunosine]

Sure, so 64fa37f -f7435b6 were my initial commits/getting my branch caught up with master. 5f593f5 was dealing with an error that reported no setuptools. After no setuptools, it said that it didn't have python >=3.6. 6d40956 I didn't want to try to manage python installations through apt-get so I switched over to Neurodocker's conda option. This failed tests because it couldn't find nibabel. c52c7f0 I realized that we were manually setting PATH, which was overwriting the path we set for FSL, as well as the path to the conda default environment, so I added those paths to PATH we set manually. Additionally, I realized that I couldn't search for the acq and rec tags separately, because in the case when neither of those is provided I was still adding an extra underscore to the glob, which meant it didn't find any T1s. This now worked in tests up till the point that freesurfer's aparcstats2table was called, when it threw an error because it couldn't find the csv module. c0cfb22 aparcstats2table is written in python 2.7. Apparently, while python2 does ship with ubuntu xenial, they've stripped out part of the standard lib including the csv module and put it in libpython2.7-stdlib, so I had to add that to the apt installs. Now we're managing python 3 with conda and python 2 using apt, but at least that saves us from having to switch between multiple conda environments.

That summarizes all of the issues I ran into. Happy to make any changes if you think there's a better way to solve some of those problems.

[alexlicohen]

This is wonderful work; thank you for putting in the effort!

On Jun 30, 2020, at 10:05 AM, Dylan Nielson notifications@github.com wrote:

Sure, so 64fa37f https://github.com/BIDS-Apps/freesurfer/pull/60/commits/64fa37f9d49d869cbd8932f9b9dd0b57def13028 -f7435b6 https://github.com/BIDS-Apps/freesurfer/pull/60/commits/f7435b6d31933389d29cccf75692c940a6f9ef92 were my initial commits/getting my branch caught up with master. 5f593f5 https://github.com/BIDS-Apps/freesurfer/pull/60/commits/5f593f5e5a594947348a613870a3eed5b287f7cd was dealing with an error that reported no setuptools. After no setuptools, it said that it didn't have python >=3.6. 6d40956 https://github.com/BIDS-Apps/freesurfer/pull/60/commits/6d40956ce349f94cf3ef88b23b36ef4fb01af6d2 I didn't want to try to manage python installations through apt-get so I switched over to Neurodocker's conda option. This failed tests because it couldn't find nibabel. c52c7f0 https://github.com/BIDS-Apps/freesurfer/pull/60/commits/c52c7f056d8b083f5d8e056e3951bec4b9a62c2d I realized that we were manually setting PATH, which was overwriting the path we set for FSL, as well as the path to the conda default environment, so I added those paths to PATH we set manually. Additionally, I realized that I couldn't search for the acq and rec tags separately, because in the case when neither of those is provided I was still adding an extra underscore to the glob, which meant it didn't find any T1s. This now worked in tests up till the point that freesurfer's aparcstats2table was called, when it threw an error because it couldn't find the csv module. c0cfb22 https://github.com/BIDS-Apps/freesurfer/pull/60/commits/c0cfb226728f3bf38225d495c4da3f7ff5732ee1 aparcstats2table is written in python 2.7. Apparently, while python2 does ship with ubuntu xenial, they've stripped out part of the standard lib including the csv module and put it in libpython2.7-stdlib, so I had to add that to the apt installs. Now we're managing python 3 with conda and python 2 using apt, but at least that saves us from having to switch between multiple conda environments.

That summarizes all of the issues I ran into. Happy to make any changes if you think there's a better way to solve some of those problems.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/BIDS-Apps/freesurfer/pull/60#issuecomment-651813229, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACB5V74Y3K64C2AULYPQFITRZHWKXANCNFSM4OIW2MRQ.

[PeerHerholz]

Hi @Shotgunosine,

awesome, thank you very much for this detailed description, some real detective work! I think you tackled it the right way and the image won't get super big as a result, hence it should be ok. Thanks again for putting the hard work in, this is a great improvement. I'll go ahead and merge it.