Skip to content

FIX: Remove trace by default #293

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

Merged
merged 4 commits into from
Dec 9, 2014
Merged

Conversation

larsoner
Copy link
Contributor

@larsoner larsoner commented Dec 5, 2014

Allows for discarding the trace, and making it the default behavior. It generally seems to be nuisance (non-)data in the analysis process.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling bafe40e on Eric89GXL:remove-trace into c54080f on nipy:master.

@matthew-brett
Copy link
Member

A test to check you removed the correct volume?

@matthew-brett
Copy link
Member

No problem with putting in the release. I'm still writing docs, so another day suits me OK.

@larsoner
Copy link
Contributor Author

larsoner commented Dec 5, 2014

Cool. I'll try throwing Appveyor in here, too. :)

@larsoner larsoner force-pushed the remove-trace branch 3 times, most recently from a29e623 to eade99f Compare December 5, 2014 20:01
@larsoner larsoner mentioned this pull request Dec 5, 2014
@larsoner
Copy link
Contributor Author

larsoner commented Dec 5, 2014

Okay, Travis and Appveyor are happy, and I made the test more comprehensive. Ready for review/merge from my end. The badges I added to the README should update after merge. If you don't want them there, I can remove them.

@matthew-brett
Copy link
Member

I think the install_python.ps1 file should go in tools rather than nibabel-data.

Any way we could submodule that file to keep up to date with Olivier's changes?

A bit 50:50 on the travis badge, because travis-ci is so unreliable, can fail and give us bad press when there's nothing wrong.

@larsoner
Copy link
Contributor Author

larsoner commented Dec 8, 2014

Hah:

https://github.com/blog/1935-see-results-from-all-pull-request-status-checks

I'll undo github-multi-status while I'm at it...

@larsoner
Copy link
Contributor Author

larsoner commented Dec 8, 2014

I trimmed the install_python.ps1 file down a bit from what they had originally. One problem with submoduling this one is that they could switch to using system python instead of miniconda (which was an option in their original version), which would screw up our appveyor.yml file commands :(

@larsoner
Copy link
Contributor Author

larsoner commented Dec 8, 2014

WDYT about keeping the Coveralls badge? 94% is awesome. But I guess if Travis goes wrong, you might get that one messed up, too...

@matthew-brett
Copy link
Member

Coveralls badge is fine, totally agree about the 94% !

Do you think it is possible to get the appveyor commands working for Python.org Python? It's the Python most people are likely to be using...

@larsoner
Copy link
Contributor Author

larsoner commented Dec 8, 2014

Yeah, the drawback is substantially increased testing time because numpy, scipy, etc. will need to be built. Or we'll have to set up wheels, which I don't really know how to do. Maybe leave that for a subsequent PR when you're feeling bored? :)

@matthew-brett
Copy link
Member

I can't practically futz with the web stuff from here I'm afraid, but sure, miniconda considerably better than nothing, thanks again. Last I heard on this :

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 9841c41 on Eric89GXL:remove-trace into cf4e25e on nipy:master.

@larsoner
Copy link
Contributor Author

larsoner commented Dec 8, 2014

Alright, Travis and Appveyor are happy, and I kept only the Coveralls badge. Ready for review/merge from my end.

@matthew-brett
Copy link
Member

Thanks, merging.

matthew-brett added a commit that referenced this pull request Dec 9, 2014
FIX: Remove trace by default

Allows for discarding the trace, and making it the default behavior. It generally seems to be nuisance (non-)data in the analysis process.

Add appveyor build and badge.
@matthew-brett matthew-brett merged commit adb15a3 into nipy:master Dec 9, 2014
grlee77 pushed a commit to grlee77/nibabel that referenced this pull request Mar 15, 2016
FIX: Remove trace by default

Allows for discarding the trace, and making it the default behavior. It generally seems to be nuisance (non-)data in the analysis process.

Add appveyor build and badge.
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.

3 participants