-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Finish removing nose #7935
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
Finish removing nose #7935
Conversation
d7cdee2
to
86a498e
Compare
Python 2 failure is expected, but I'm not really sure what's up with Windows. Thoughts, @Kojoley? |
@@ -516,6 +515,7 @@ def skip_if_command_unavailable(cmd): | |||
try: | |||
check_output(cmd) | |||
except: | |||
return skipif(True, reason='missing command: %s' % cmd[0]) | |||
import pytest | |||
pytest.skip(reason='missing command: %s' % cmd[0]) |
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.
This is wrong, but I'll fix it in the next update.
Hi @QuLogic |
It was only required with nose because it needed a Matplotlib-provided plugin for nose.
It's a compatibility wrapper around nose/pytest, but has never been released.
They've never been released and are no longer used.
Never been in a release and we don't want people depending on it.
Also, privatize all internal users which have never been released.
86a498e
to
c4a19c1
Compare
Because pytest requires some additional changes to get Python 2 working, I've scaled this back to just be about removing nose. I'll open the Py2 stuff in another PR shortly. Ping @NelleV. |
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.
LGTM 👍
@@ -101,7 +101,7 @@ def test_image_python_io(): | |||
plt.imread(buffer) | |||
|
|||
|
|||
@knownfailureif(not HAS_PIL) | |||
@needs_pillow |
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.
minor nitpick: I prefer the explicit command.
The tests did not run on 2.7 3.4 and 3.5 (rather only the few vestigial tests that nose finds did run -- https://travis-ci.org/matplotlib/matplotlib/jobs/196044519#L1714). I think you need to update whatever travis runs before we can merge this. |
@@ -181,7 +180,7 @@ def process_image(self, padded_src, dpi): | |||
return t2 | |||
|
|||
if LooseVersion(np.__version__) < LooseVersion('1.7.0'): | |||
skip('Disabled on Numpy < 1.7.0') | |||
pytest.skip('Disabled on Numpy < 1.7.0') |
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 this block can be removed as we require numpy>=1.7.1 now.
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.
Removed now.
Tests pass locally for me on Py2 and Py3, with a few exceptions that @QuLogic says (on gitter) are known, and can certainly be fixed later. So I'll merge now as the current situation (effectively no CI on Py2) is much worse :-) |
I tried to keep anything in
testing
compatible with both nose and pytest for now, but anything new has been either removed/replaced or privatized so that people don't depend on it any further.It works locally on Python 3, but let's see how CI likes it.