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

fix unintentional skipped tests #1557

Merged
merged 10 commits into from
Oct 4, 2017
132 changes: 34 additions & 98 deletions xarray/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from contextlib import contextmanager
from distutils.version import LooseVersion
import re
import importlib

import numpy as np
from numpy.testing import assert_array_equal
Expand All @@ -15,6 +16,20 @@
from xarray.core.pycompat import PY3
from xarray.testing import assert_equal, assert_identical, assert_allclose


def _importorskip(modname, minversion=None):
try:
mod = importlib.import_module(modname)
has = True
Copy link
Member

Choose a reason for hiding this comment

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

put everything from here on down an the else clause

if minversion is not None:
if LooseVersion(mod.__version__) < LooseVersion(minversion):
raise ImportError('Minimum version not satisfied')
except ImportError:
has = False
return (has, pytest.mark.skipif((not has),
reason='requires {}'.format(modname)))


try:
import unittest2 as unittest
except ImportError:
Expand All @@ -25,111 +40,32 @@
except ImportError:
import mock

try:
import scipy
has_scipy = True
except ImportError:
has_scipy = False

try:
import pydap.client
has_pydap = True
except ImportError:
has_pydap = False

try:
import netCDF4
has_netCDF4 = True
except ImportError:
has_netCDF4 = False


try:
import h5netcdf
has_h5netcdf = True
except ImportError:
has_h5netcdf = False


try:
import Nio
has_pynio = True
except ImportError:
has_pynio = False

has_matplotlib, requires_matplotlib = _importorskip('matplotlib')
has_scipy, requires_scipy = _importorskip('scipy')
has_pydap, requires_pydap = _importorskip('pydap.client')
has_netCDF4, requires_netCDF4 = _importorskip('netCDF4')
has_h5netcdf, requires_h5netcdf = _importorskip('h5netcdf')
has_pynio, requires_pynio = _importorskip('pynio')
has_dask, requires_dask = _importorskip('dask')
has_bottleneck, requires_bottleneck = _importorskip('bottleneck', '1.0')
Copy link
Member

Choose a reason for hiding this comment

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

bottleneck 1.0 came out in Feb 2015. I'm not sure it's worth having the version check here.

has_rasterio, requires_rasterio = _importorskip('rasterio')
has_pathlib, requires_pathlib = _importorskip('pathlib')

# some special cases
has_scipy_or_netCDF4 = has_scipy or has_netCDF4
requires_scipy_or_netCDF4 = pytest.mark.skipif(
not has_scipy_or_netCDF4, reason='requires scipy or netCDF4')
if not has_pathlib:
has_pathlib, requires_pathlib = _importorskip('pathlib2')

try:
import dask.array
if has_dask:
import dask
dask.set_options(get=dask.get)
has_dask = True
except ImportError:
has_dask = False


try:
import matplotlib
has_matplotlib = True
except ImportError:
has_matplotlib = False


try:
import bottleneck
if LooseVersion(bottleneck.__version__) < LooseVersion('1.0'):
raise ImportError('Fall back to numpy')
has_bottleneck = True
except ImportError:
has_bottleneck = False

try:
import rasterio
has_rasterio = True
except ImportError:
has_rasterio = False

try:
import pathlib
has_pathlib = True
except ImportError:
try:
import pathlib2
has_pathlib = True
except ImportError:
has_pathlib = False


# slighly simpler construction that the full functions.
# Generally `pytest.importorskip('package')` inline is even easier
requires_matplotlib = pytest.mark.skipif(
not has_matplotlib, reason='requires matplotlib')
requires_scipy = pytest.mark.skipif(
not has_scipy, reason='requires scipy')
requires_pydap = pytest.mark.skipif(
not has_pydap, reason='requires pydap')
requires_netCDF4 = pytest.mark.skipif(
not has_netCDF4, reason='requires netCDF4')
requires_h5netcdf = pytest.mark.skipif(
not has_h5netcdf, reason='requires h5netcdf')
requires_pynio = pytest.mark.skipif(
not has_pynio, reason='requires pynio')
requires_scipy_or_netCDF4 = pytest.mark.skipif(
not has_scipy and not has_netCDF4, reason='requires scipy or netCDF4')
requires_dask = pytest.mark.skipif(
not has_dask, reason='requires dask')
requires_bottleneck = pytest.mark.skipif(
not has_bottleneck, reason='requires bottleneck')
requires_rasterio = pytest.mark.skipif(
not has_rasterio, reason='requires rasterio')
requires_pathlib = pytest.mark.skipif(
not has_pathlib, reason='requires pathlib / pathlib2'
)


try:
_SKIP_FLAKY = not pytest.config.getoption("--run-flaky")
_SKIP_NETWORK_TESTS = not pytest.config.getoption("--run-network-tests")
except ValueError:
except (ValueError, AttributeError):
# Can't get config from pytest, e.g., because xarray is installed instead
# of being run from a development version (and hence conftests.py is not
# available). Don't run flaky tests.
Expand Down
4 changes: 2 additions & 2 deletions xarray/tests/test_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -1508,8 +1508,6 @@ def test_dask(self):
self.assertDatasetEqual(actual, expected)


@requires_scipy
@requires_pynio
class TestPyNio(CFEncodedDataTest, Only32BitTypes, TestCase):
def test_write_store(self):
# pynio is read-only for now
Expand All @@ -1529,6 +1527,8 @@ def roundtrip(self, data, save_kwargs={}, open_kwargs={},
autoclose=self.autoclose, **open_kwargs) as ds:
yield ds

@requires_scipy
@requires_pynio
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is enough -- the other methods defined on super-classes should also need pynio to pass for this TestCase. Looking at the test log on Travis, I see a lot of xfail and XPASS test cases, so maybe they are getting disabled by something else?

Copy link
Member Author

Choose a reason for hiding this comment

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

totally agree, this move does fix most of the issues but there are some additional issues. Now that we know what the problem is and have confirmation that this is a bug from the pytest team, I'll work on a more robust solution.

def test_weakrefs(self):
example = Dataset({'foo': ('x', np.arange(5.0))})
expected = example.rename({'foo': 'bar', 'x': 'y'})
Expand Down