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

Refactor Fit #1

Merged
merged 2 commits into from
May 14, 2018
Merged

Refactor Fit #1

merged 2 commits into from
May 14, 2018

Conversation

TomMaullin
Copy link

Hi, I'm not sure if this is too late but I have refactored the code mentioned in issue ME-ICA#25 on the ME-ICA repository. I'm afraid I have run out of time before managing to test it but perhaps this will still save someone a few minutes at some point!

@tsalo
Copy link
Owner

tsalo commented May 11, 2018

@TomMaullin thanks so much for this PR. It looks pretty good to me, although there are a couple of things I'd like to tweak. Unfortunately, between updates to my fork and who knows what else, GitHub is being less than cooperative. Since I'm not very proficient with command line git (which seems to be necessary to resolve the current conflicts), would you mind pulling from my current master to eliminate those conflicts?

TomMaullin pushed a commit to TomMaullin/tedana that referenced this pull request May 14, 2018
Patches:
* csstepdata saved as JSON rather than a text dump
* Removes unused imports in test.utils to fix linting errors
@TomMaullin
Copy link
Author

TomMaullin commented May 14, 2018

Hi @tsalo ,

Sure! I've merged this with the master! Again I didn't have time to get this code running and test how well it worked but hopefully, as my edits were quite simple, it should be quite easy to see where I have made them and, if there are any errors, hopefully * touch wood * they shouldn't be too difficult to resolve.

@tsalo
Copy link
Owner

tsalo commented May 14, 2018

@TomMaullin This looks great, thanks! I'll merge now and check the CI results after. There's always the possibility we'll need to squash a bug or two, but the code makes sense to me.

@tsalo tsalo merged commit 6cc8f49 into tsalo:master May 14, 2018
tsalo pushed a commit that referenced this pull request Nov 1, 2018
tsalo pushed a commit that referenced this pull request Sep 13, 2019
tsalo pushed a commit that referenced this pull request Sep 21, 2019
tsalo pushed a commit that referenced this pull request Aug 21, 2020
[ENH,FIX] Fixing decision tree workflow
tsalo pushed a commit that referenced this pull request Dec 30, 2020
Changed default compute_zvalues of get_coeffs to False
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.

2 participants