Skip to content
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

Merged
merged 7 commits into from
Jan 28, 2017
Merged

Finish removing nose #7935

merged 7 commits into from
Jan 28, 2017

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Jan 24, 2017

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.

@QuLogic QuLogic added this to the 2.1 (next point release) milestone Jan 24, 2017
@QuLogic QuLogic force-pushed the pytest-remove-nose branch 2 times, most recently from d7cdee2 to 86a498e Compare January 24, 2017 08:55
@QuLogic
Copy link
Member Author

QuLogic commented Jan 24, 2017

Python 2 failure is expected, but I'm not really sure what's up with Windows. Thoughts, @Kojoley?
Maybe not bother with the tests.py there?

@@ -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])
Copy link
Member Author

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.

@NelleV
Copy link
Member

NelleV commented Jan 27, 2017

Hi @QuLogic
Can you ping me when you are done on this?

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.
@QuLogic QuLogic changed the title Finish removing nose [MRG] Finish removing nose Jan 28, 2017
@QuLogic
Copy link
Member Author

QuLogic commented Jan 28, 2017

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.

Copy link
Member

@NelleV NelleV left a 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
Copy link
Member

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.

@NelleV NelleV changed the title [MRG] Finish removing nose [MRG+1] Finish removing nose Jan 28, 2017
@anntzer
Copy link
Contributor

anntzer commented Jan 28, 2017

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.
(Or perhaps that can be a separate PR but I'd like to have that go in first then.)

@@ -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')
Copy link
Contributor

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed now.

@anntzer
Copy link
Contributor

anntzer commented Jan 28, 2017

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 :-)

@anntzer anntzer merged commit 7bcaa51 into matplotlib:master Jan 28, 2017
@QuLogic QuLogic deleted the pytest-remove-nose branch January 28, 2017 23:37
@QuLogic QuLogic changed the title [MRG+1] Finish removing nose Finish removing nose Jan 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants