TST: Add GIFTI tests #355
TST: Add GIFTI tests #355
Conversation
|
@effigies besides the other two, I also rebased this one. Anything you'd like to see here? It's just adding a bit to test coverage. |
| @@ -453,7 +453,7 @@ def add_gifti_data_array(self, dataarr): | |||
| if isinstance(dataarr, GiftiDataArray): | |||
| self.darrays.append(dataarr) | |||
| else: | |||
| print("dataarr paramater must be of tzpe GiftiDataArray") | |||
| raise ValueError("dataarr paramater must be of type GiftiDataArray") | |||
|
|
|||
There was a problem hiding this comment.
Actually, while I'm critiquing, my preference on this idiom would be:
if not isinstance(...):
raise TypeError(...)
self.darrays.append(dataarr)
Just associates the check more closely with the guarded case, and avoids visually demoting the useful code by putting it in an else block.
There was a problem hiding this comment.
Good call on the style and error type. Fixed, and scrubbed the code for other ValueErrors that should be TypeErrors. Tests running.
|
@effigies I'm seeing an AppVeyor failure in this PR. I tried to follow it backwards to previous PRs, but none completed it. I also didn't reproduce on my local config. What's AppVeyor? And do we care? Previous PRs were merged without AppVeyor finishing. Edit: appveyor failure is here: |
|
It does Windows builds, at least for nibabel. I left a note on #358. 32 bit builds are broken under numpy 1.10, I believe for floating point precision issues. That's beyond my skill/time to resolve, so as long as that's the only error, I'm comfortable merging. On October 17, 2015 8:33:16 AM EDT, Ben Cipollini [email protected] wrote:
Chris Markiewicz |
| else: | ||
| print("dataarr paramater must be of tzpe GiftiDataArray") | ||
| if not isinstance(dataarr, GiftiDataArray): | ||
| raise TypeError("dataarr paramater must be of type GiftiDataArray") |
There was a problem hiding this comment.
For consistency, how about "Not a valid GiftiDataArray instance"?
|
Noting for posterity that although these |
TST: Add GIFTI tests RF: Raise TypeErrors on mistyped arguments
TST: Add GIFTI tests RF: Raise TypeErrors on mistyped arguments
This isn't complete, but add some basic smoke tests (at least
print_summaryshouldn't raise an error), that explicit errors are hit, and that non-Nonedefaults are set by objects.