Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Finish removing nose#7935
Uh oh!
There was an error while loading.Please reload this page.
Conversation
d7cdee2 to86a498eCompareQuLogic commentedJan 24, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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-providedplugin for nose.
It's a compatibility wrapper around nose/pytest, but has never beenreleased.
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.
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.
anntzer commentedJan 28, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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
testingcompatible with both nose and pytest for now, but anythingnew 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.