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

Added mask shape check in tenfit #403

Merged
merged 2 commits into from Jul 31, 2014

Conversation

Projects
None yet
3 participants
@samuelstjean
Contributor

samuelstjean commented Jul 31, 2014

Seems like the old code wasn't checking if the mask was the same shape as the data, so I stole the snippet in peaks_from_model.

If the mask could be broadcasted to a valid shape, the code would run and give random results, not the maks and the data must be identical for the fit method to run.

@arokem

This comment has been minimized.

Member

arokem commented Jul 31, 2014

Nice. Could you please write a simple test for this (using test_raises)?

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented Jul 31, 2014

Where can I find an example of how to do that?

@omarocegueda

This comment has been minimized.

Contributor

omarocegueda commented Jul 31, 2014

Hi,
you just need to import assert_raises from numpy.testing, and then call:

assert_raises([ExceptionTypeExpected], [funtion to be called, or any
callable], [arguments])

for example:

https://github.com/omarocegueda/dipy/blob/4af588b490662ead565bcc16393ff2ee64565812/dipy/align/tests/test_metrics.py,

I hope it helps

On Thu, Jul 31, 2014 at 9:15 AM, Samuel St-Jean notifications@github.com
wrote:

Where can I find an example of how to do that?


Reply to this email directly or view it on GitHub
#403 (comment).

"Cada quien es dueño de lo que calla y esclavo de lo que dice"
-Proverbio chino.
"We all are owners of what we keep silent and slaves of what we say"
-Chinese proverb.

http://www.cimat.mx/~omar

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented Jul 31, 2014

Seems to be working now, thanks.

arokem added a commit that referenced this pull request Jul 31, 2014

Merge pull request #403 from samuelstjean/fix_tenfit_mask
Added mask shape check in tenfit

@arokem arokem merged commit 4d0a67c into nipy:master Jul 31, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@samuelstjean samuelstjean deleted the samuelstjean:fix_tenfit_mask branch Jul 31, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment