Skip to content

fix unintentional skipped tests #1557

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.

Sign up for GitHub

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

Merged
jhamman merged 10 commits into from
Oct 4, 2017
Merged

fix unintentional skipped tests #1557

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e98bf36
move requires_pynio to fix GH1531, refactor tests/__init__.py
Sep 7, 2017
36ad7c3
remove ModuleNotFoundError
Sep 7, 2017
8768d65
no pytest decorators on classes with multiple inheritance in test_bac…
Sep 7, 2017
18e01c9
wrap class decorators for tests
Sep 7, 2017
0220da5
Merge branch 'master' of github.com:pydata/xarray into fix/1531
Oct 2, 2017
27b635b
no more class decorators in test_backends
Oct 3, 2017
5b4c966
whats new and some more cleanup
Oct 3, 2017
5f73351
whatsnew update
Oct 3, 2017
6651516
use unittest skip decorators, way easier
Oct 4, 2017
993992d
update workaround comment
Oct 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 34 additions & 98 deletions xarray/tests/__init__.py
View file
Open in desktop
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

@shoyer shoyer Oct 3, 2017

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

@shoyer shoyer Sep 7, 2017

Copy link
Copy Markdown
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
View file
Open in desktop
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

@shoyer shoyer Sep 7, 2017

Copy link
Copy Markdown
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?

@jhamman jhamman Sep 7, 2017

Copy link
Copy Markdown
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