-
Notifications
You must be signed in to change notification settings - Fork 264
MRG: Cleaner tests #254
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
MRG: Cleaner tests #254
Conversation
@@ -55,7 +55,7 @@ the array. | |||
>>> slice_1 = epi_img_data[:, 30, :] | |||
>>> slice_2 = epi_img_data[:, :, 16] | |||
>>> show_slices([slice_0, slice_1, slice_2]) | |||
>>> plt.suptitle("Center slices for EPI image") | |||
>>> plt.suptitle("Center slices for EPI image") # doctest: +SKIP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These skips were necessary on my system or tests would fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens on your system for these? Presumably you are running make doctests
on the docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let me test again on current master
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g.:
larsoner@bunk:~/custombuilds/nibabel$ make testmanual
F..F
======================================================================
FAIL: Doctest: coordinate_systems.rst
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python2.7/doctest.py", line 2226, in runTest
raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for coordinate_systems.rst
File "/home/larsoner/custombuilds/nibabel/doc/source/coordinate_systems.rst", line 0
----------------------------------------------------------------------
File "/home/larsoner/custombuilds/nibabel/doc/source/coordinate_systems.rst", line 58, in coordinate_systems.rst
Failed example:
plt.suptitle("Center slices for EPI image")
Expected nothing
Got:
<matplotlib.text.Text object at 0x2b0f27e16790>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try adding a semicolon at the end of the line instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should indeed suppress it, I'll add it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...actually it does not suppress it. Back to the SKIP
?
@matthew-brett if you are happy with this, it would be good to get this in sooner rather than later since it touches a lot of files. |
@@ -304,7 +304,7 @@ def get_affine(self): | |||
# column, slice) | |||
vox = self.voxel_sizes | |||
ipp = self.image_position | |||
if None in (orient, vox, ipp): | |||
if orient is None or vox is None or ipp is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe any(p is None for p in (orient, vox, ipp))
as below, for beauty?
Rebased for good measure. Only the |
Latest commit adds the |
OK - thanks for that - will merge when the travis tests are done. |
Cool, looks like that's already happened :) |
MRG: Cleaner tests In the process of working on #253 I was a little bit surprised by the number of warnings during testing. This cleans that up, and fixes a couple minor errors along the way.
MRG: Cleaner tests In the process of working on nipy#253 I was a little bit surprised by the number of warnings during testing. This cleans that up, and fixes a couple minor errors along the way.
In the process of working on #253 I was a little bit surprised by the number of warnings during testing. This cleans that up, and fixes a couple minor errors along the way.
Ready for review/merge.