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

Set strict xfail option #5220

Merged
merged 12 commits into from Aug 16, 2019

Conversation

@jrbourbeau
Copy link
Member

commented Aug 4, 2019

This PR is intended to fix #5120. I'll incrementally go through and set strict=False appropriately

  • Tests added / passed
  • Passes black dask / flake8 dask
@jrbourbeau

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

Things are passing now with strict=True by default (cc @TomAugspurger). Summarizing the changes in this PR:

  • Most of the changes here are removing old xfails that have since been fixed and are now passing

  • Added strict=False to test_deep_bases_win_over_dependents in dask/tests/test_order.py. I think this test is supposed to fail for some graphs, but not others. It would be great to get another set of eyes on that test.

  • Added strict=False to test_Array_numpy_gufunc_call__array_ufunc__01 and test_Array_numpy_gufunc_call__array_ufunc__02 in dask/array/tests/test_array_core.py. These test gufuncs from the private _umath_linalg module in NumPy. Currently, test_Array_numpy_gufunc_call__array_ufunc__02 fails due to an issue with meta (see traceback below) which went unnoticed because both these tests were marked as xfail. Will open up separate issue to track this.

Traceback for

dask/array/tests/test_array_core.py::test_Array_numpy_gufunc_call__array_ufunc__02:

==================================================== FAILURES =====================================================
__________________________________ test_Array_numpy_gufunc_call__array_ufunc__02 __________________________________

    @pytest.mark.skipif(
        LooseVersion(np.__version__) < "1.14.0",
        reason="NumPy doesn't have `np.linalg._umath_linalg` yet",
    )
    # @pytest.mark.xfail(reason="Protect from `np.linalg._umath_linalg.eig` breaking")
    def test_Array_numpy_gufunc_call__array_ufunc__02():
        x = da.random.normal(size=(3, 10, 10), chunks=(2, 10, 10))
        nx = x.compute()
        nw, nv = np.linalg._umath_linalg.eig(nx)
>       w, v = np.linalg._umath_linalg.eig(x, output_dtypes=(float, float))

dask/array/tests/test_array_core.py:262:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
dask/array/core.py:1122: in __array_ufunc__
    numpy_ufunc, numpy_ufunc.signature, *inputs, **kwargs
dask/array/gufunc.py:481: in apply_gufunc
    meta = meta_from_array(meta, len(output_shape), dtype=odt)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

x = array([], shape=(0, 0), dtype=complex128), ndim = 2, dtype = <class 'float'>

    def meta_from_array(x, ndim=None, dtype=None):
        """ Normalize an array to appropriate meta object

        Parameters
        ----------
        x: array-like, callable
            Either an object that looks sufficiently like a Numpy array,
            or a callable that accepts shape and dtype keywords
        ndim: int
            Number of dimensions of the array
        dtype: Numpy dtype
            A valid input for ``np.dtype``

        Returns
        -------
        array-like with zero elements of the correct dtype
        """
        # If using x._meta, x must be a Dask Array, some libraries (e.g. zarr)
        # implement a _meta attribute that are incompatible with Dask Array._meta
        if hasattr(x, "_meta") and isinstance(x, Array):
            x = x._meta

        if dtype is None and x is None:
            raise ValueError("You must specify the meta or dtype of the array")

        if np.isscalar(x):
            x = np.array(x)

        if x is None:
            x = np.ndarray

        if isinstance(x, type):
            x = x(shape=(0,) * (ndim or 0), dtype=dtype)

        if (
            not hasattr(x, "shape")
            or not hasattr(x, "dtype")
            or not isinstance(x.shape, tuple)
        ):
            return x

        if isinstance(x, list) or isinstance(x, tuple):
            ndims = [
                0
                if isinstance(a, numbers.Number)
                else a.ndim
                if hasattr(a, "ndim")
                else len(a)
                for a in x
            ]
            a = [a if nd == 0 else meta_from_array(a, nd) for a, nd in zip(x, ndims)]
            return a if isinstance(x, list) else tuple(x)

        if ndim is None:
            ndim = x.ndim

        try:
            meta = x[tuple(slice(0, 0, None) for _ in range(x.ndim))]
            if meta.ndim != ndim:
                if ndim > x.ndim:
                    meta = meta[(Ellipsis,) + tuple(None for _ in range(ndim - meta.ndim))]
                    meta = meta[tuple(slice(0, 0, None) for _ in range(meta.ndim))]
                elif ndim == 0:
                    meta = meta.sum()
                else:
                    meta = meta.reshape((0,) * ndim)
        except Exception:
            meta = np.empty((0,) * ndim, dtype=dtype or x.dtype)

        if np.isscalar(meta):
            meta = np.array(meta)

        if dtype and meta.dtype != dtype:
>           meta = meta.astype(dtype)
E           numpy.ComplexWarning: Casting complex values to real discards the imaginary part

dask/array/utils.py:105: ComplexWarning
@TomAugspurger
Copy link
Member

left a comment

Great work @jrbourbeau.

dask/tests/test_order.py Show resolved Hide resolved

@jrbourbeau jrbourbeau merged commit 1f88b46 into dask:master Aug 16, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jrbourbeau jrbourbeau deleted the jrbourbeau:xfail-strict branch Aug 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.