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

V0.4.x #8

Open
wants to merge 74 commits into
base: master
Choose a base branch
from
Open

V0.4.x #8

wants to merge 74 commits into from

Conversation

rtrhd
Copy link

@rtrhd rtrhd commented Mar 18, 2017

Hi,

I've been maintaining an optimised python2/3 fork of ICA-AROMA for use in the Bristol University CRIC imaging centre for a while now. I've also added some regression tests against your current version, updated the documentation and made it installable as a python package. You may well not want to consider merging this branch as it stands since the commit history could do with squashing and there is quite a large change in the layout of the repo but let me know if it is any use to you and I can squash/rebase if required.

All the Best,
Ron Hartley-Davies

…rng seed option to allow regression testing.
…e; Use setuptools to install executable script; Add manual install with Makefile
@maartenmennes
Copy link
Owner

Holy Moly Ron, that's some serious work that you've put in! Thanks for sharing! We'll need a bit of time to parse your improvements - also this is the first time I'm dealing with such an extensive pull request ;)

shutil.copytree() doesn't handle single files so treat files and directories separately.
Avoid some of the bloat in the docker image mainly by not installing fsl recommended dependencies .
@maartenmennes
Copy link
Owner

Implemented the feature_time_series refactoring. Great speed improvement associated. Many thanks!

@anibalsolon
Copy link

Hi @maartenmennes and @rtrhd ,

is there a roadmap to merge this PR? I believe it is awesome work from @rtrhd , but I am not sure if it is finished.

Please let me know if I can help developing/testing to merge it to the main repo!

Thank you

@maartenmennes
Copy link
Owner

Hi @anibalsolon, many thanks for your input. I too believe it's great work by @rtrhd, but the changes are so massive that I really need to take my time to go through them. Unfortunately, I don't see that planned for the immediate future...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants