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

Switch to pytest #10856

Closed
7 of 8 tasks
charris opened this issue Apr 6, 2018 · 24 comments
Closed
7 of 8 tasks

Switch to pytest #10856

charris opened this issue Apr 6, 2018 · 24 comments

Comments

@charris
Copy link
Member

charris commented Apr 6, 2018

  • use pytest markers
  • use pytest for runtests and ci tests.
  • remove the pytest_tools directory.
  • remove run_module_suite from the test files.
  • fixup the remaining yield tests, warnings currently suppressed in ci testing.
  • test for nose dependencies by removing nose import from some ci tests.
  • update documentation
  • update numpy-wheels
@charris charris added this to the 1.15.0 release milestone Apr 6, 2018
@hameerabbasi
Copy link
Contributor

fixup the remaining yield tests, warnings currently suppressed in ci testing.

I'm willing to work on this, if someone else isn't actively working on it.

@charris
Copy link
Member Author

charris commented Apr 9, 2018

@hameerabbasi It's done, just not merged.

@mattip
Copy link
Member

mattip commented Apr 9, 2018

Is there anything I can help with?

@charris
Copy link
Member Author

charris commented Apr 9, 2018

@mattip It's pretty much done and I'm about 1/2 through updating the documentation. Changing some of the ci tests to not import nose could be helpful. Nose is still used, when available, to test the nose decorators we are keeping around for downstream folks who still rely on the NumPy nose testing framework.

@charris
Copy link
Member Author

charris commented Apr 9, 2018

If you are looking for other things to do for the first day on the job, I would like to branch 1.15 fairly soon, so PR's with the 1.15 milestone could use review.

@mattip
Copy link
Member

mattip commented Apr 15, 2018

A couple of things I noticed, are they issues that need to be cleaned up?

  • running pytest in the numpy checkout raises an ImportError, does it need a tweak in conftest.py?
  • trying to specify a single test class now requires two colons in runtests.py -t numpy/lib/tests/test_polynomial.py:TestDocs where previously only one was required
  • running pytest outside the numpy checkout after a pip install . works, but does not skip slow tests by default AFAICT (trying the same test_polynomial.py from above), and I could not work out how to make it skip the slow tests

@pv
Copy link
Member

pv commented Apr 15, 2018

The first one: does doing an in-place build first help?

The third: can the default pytest selection markers be configured in conftest.py or only in the ini file?
I don't know how to do this either.

@mattip
Copy link
Member

mattip commented Apr 15, 2018

About running pytest without runtests.py

For this first version with pytest, I think it is OK to still require runtests.py, this seems hard to fix since the error happens before numpy.conftest.py is read

here is what I did

  • git clean -xfd
  • python runtests.py
  • NumPy builds cleanly and runs all tests
  • python -mpytest => error (below)
  • additionally try python setup.py build then rerunning pytest, still the same error
Traceback (most recent call last):
  File "/home/matti/pypy_stuff/cpython_virt/local/lib/python2.7/site-packages/_pytest/config.py", line 371, in _importconftest
    mod = conftestpath.pyimport()
  File "/home/matti/pypy_stuff/cpython_virt/local/lib/python2.7/site-packages/py/_path/local.py", line 668, in pyimport
    __import__(modname)
  File "/home/matti/pypy_stuff/numpy/numpy/__init__.py", line 131, in <module>
    raise ImportError(msg)
ImportError: Error importing numpy: you should not try to import numpy from
        its source directory; please exit the numpy source tree, and relaunch
        your python interpreter from there.
ERROR: could not load /home/matti/pypy_stuff/numpy/numpy/conftest.py

@pv
Copy link
Member

pv commented Apr 15, 2018

That error message goes away after python setup.py build_ext -i, it's meant to just catch the common import failures due to missing numpy.core.multiarray etc.

I don't know how pytest is exactly intended to be used with packages that require built components.

@hameerabbasi
Copy link
Contributor

Even with pre-pytest python runtests.py, there were f2py failures if that wasn't run.

@pv
Copy link
Member

pv commented Apr 15, 2018

@hameerabbasi: not for me though, probably depends on the environment setup.

@hameerabbasi
Copy link
Contributor

hameerabbasi commented Apr 15, 2018

@pv IIRC, the tests ran fine but there was exactly one failed test regarding f2py. Maybe an intermittent failure with that particular version.

@mattip
Copy link
Member

mattip commented Apr 15, 2018

ok, so that makes my first comment a documentation problem 👍

About slow, searching for markers led me to this

pytest numpy/lib/tests/test_format.py -v -m 'not slow' which works as expected, it should probably be the default (like for the nosetests)

@mattip
Copy link
Member

mattip commented Apr 15, 2018

@hameerabbasi we digress from the topic, but for me the inplace tests run differently if I have a system-wide f2py available, that is whether I did apt install python-numpy or apt remove python-numpy

@bashtage
Copy link
Contributor

bashtage commented May 2, 2018

Noticed the README mentions nose as well.

@charris
Copy link
Member Author

charris commented Jun 8, 2018

Pushing off to 1.16 for the documentation updates.

@mattip
Copy link
Member

mattip commented Oct 25, 2018

@charris can we mark documentation done and close?

@charris
Copy link
Member Author

charris commented Oct 25, 2018

@mattip Hmm... quick review of last merge shows use of decorators that may still depend on nose. We should maybe make clear that almost all (IIRC) numpy decorators are to be avoided and replaced by pytest equivalents. The nose dependent decorators can be found in numpy/testing/tests/test_decorators.py, I think it may actually be all the numpy decorators defined in testing. So instead, use

  • pytest.mark.slow
  • pytest.mark.skip
  • pytest.mark.skipif
  • pytest.mark.xfail
  • pytest.mark.parametrize

and non-decorators

  • pytest.skip

@charris
Copy link
Member Author

charris commented Nov 17, 2018

@mattip Any update on this?

@mattip
Copy link
Member

mattip commented Nov 18, 2018

You wish to deprecate importing numpy.testing.decorators?

@charris
Copy link
Member Author

charris commented Nov 18, 2018

Yes! They were needed for Nose back in the day, but are no longer needed as pytest supplies equivalents.

@charris
Copy link
Member Author

charris commented Nov 22, 2018

Pushing this off to 1.17 as it isn't release critical, and we will catch most problems during code review.

@charris
Copy link
Member Author

charris commented May 22, 2019

@mattip I think this is done, but I don't recall the details of your documentation changes at this point.

@mattip
Copy link
Member

mattip commented May 23, 2019

Let's close this and open new issues for documentation or further transitions to native pytest semantics as needed.

@mattip mattip closed this as completed May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants