MAINT: Fix all tests #1656

thequackdaddy · 2017-08-22T03:19:37Z

Hello,

So I really like this project but it seems to have been quite quiet lately.

I spent some time today un-pinning all the packages and getting the tests to pass again. One caveat is that odo has a problem. For this, I've just odo from a repository I built. Obviously, this is not ideal.

Let me know if this is helpful. This is an amazing set of tools.

fix doc errors

msgpack seems to have moved

thequackdaddy · 2017-08-22T03:22:37Z

cc @postelrich

coveralls · 2017-08-22T03:29:01Z

Changes Unknown when pulling 16a70ec on thequackdaddy:test into ** on blaze:master**.

postelrich

@thequackdaddy Nice! This goes above and beyond my attempt to get 3.6 working. Do you want to make a PR for the odo fix? Once that is in we can try to get this merged pointing to that.

postelrich · 2017-08-22T13:52:12Z

etc/requirements.txt

@@ -1,6 +1,6 @@
 flask>=0.10.1
 flask-cors
-odo>=0.5.0
+# odo>=0.5.1


This would have to be uncommented before merge

I've made the pull request (see below).

Once you change this, I'll revert this change and push the update.

(For reasons I don't fully understand, pulling from my repo kept the version number below 0.5.0 so this kept failing :-/)

[docs] company name change from Continuum to Anaconda

fix doc errors

ehebert · 2017-10-11T20:00:56Z

blaze/expr/datetime.py

@@ -131,14 +131,14 @@ def utcfromtimestamp(expr):


 class nanosecond(DateTime): _dtype = datashape.int64
-class week(DateTime): _dtype = datashape.int16
+class week(DateTime): _dtype = datashape.int64


This change makes the tests pass; however, #1585 intentionally made the datetime types as narrow as possible.

I ran across your proposed fix while looking at the following failure when running the test py.test -s -x blaze/compute/tests/test_postgresql_compute.py::test_datetime_access[week] against current master of blaze, odo, and datashape:

obj = 'Attributes', message = 'Attribute "dtype" are different', left = dtype('int16') right = dtype('int64'), diff = None def raise_assert_detail(obj, message, left, right, diff=None): if isinstance(left, np.ndarray): left = pprint_thing(left) if isinstance(right, np.ndarray): right = pprint_thing(right) msg = """{0} are different {1} [left]: {2} [right]: {3}""".format(obj, message, left, right) if diff is not None: msg = msg + "\n[diff]: {diff}".format(diff=diff) > raise AssertionError(msg) E AssertionError: Attributes are different E E Attribute "dtype" are different E [left]: int16 E [right]: int64 /mnt/envs/blaze/lib/python3.4/site-packages/pandas/util/testing.py:1018: AssertionError

(It might make sense to break off making the aforementioned test pass into a separate issue/PR.)

This patch may checks the values, but ignores the dtypes.

$ git diff diff --git a/blaze/compute/tests/test_postgresql_compute.py b/blaze/compute/tests/test_postgresql_compe. py index 6c15703..608e8fb 100644 --- a/blaze/compute/tests/test_postgresql_compute.py +++ b/blaze/compute/tests/test_postgresql_compute.py @@ -886,4 +886,5 @@ def test_datetime_access(attr, sql_with_dts): compute(expr, sql_with_dts, return_type=pd.Series), getattr(compute(s.A, sql_with_dts, return_type=pd.Series).dt, attr), check_names=False, + check_dtype=False, )

compute(s.A, sql_with_dts, return_type=pd.Series).dt is an instance of pandas.tseries.common.DatetimeProperties which is going to return a Series with a dtype of int64 when accessing the week attribute.

#1660 applies the patch in the previous comment.

llllllllll · 2017-10-12T17:45:37Z

blaze/compute/tests/test_mongo_compute.py



 def test_summary_count(bank):
    expr = by(t.name, how_many=t.amount.count())
    result = compute(expr, bank)
-    assert result == [('Bob', 3), ('Alice', 2)]
+    assert ((result == [('Bob', 3), ('Alice', 2)]) or


We could just sort both the result and expectation

llllllllll · 2017-10-12T17:47:49Z

etc/requirements_ci.txt

-python-dateutil==2.5.3
-pytz==2016.7
-pyyaml==3.11
+bcolz


the point of this file was to pin the requirements we test with. Tests kept failing because downstream packages were being updated. I don't want to fix the whole world with every PR and it put a huge burden on whoever wanted to open the next PR.

This comment still stands

llllllllll · 2017-10-12T17:48:11Z

requirements-rtd.txt

@@ -1,7 +1,7 @@
 numpy >= 1.7
 pandas >= 0.15.0
 git+git://github.com/blaze/datashape.git
-git+git://github.com/blaze/odo.git
+git+git://github.com/thequackdaddy/odo.git@reversion


The results of the expressions compared in the test_datetime_access test do not share the same dtype. The computation which does the attribute lookup as part of the expression uses the dtypes specified in `blaze.expr.datetime`. The computation which does not do the attribute lookup returns a `DatetimeProperties` instance; attribute lookups on those instances are all `int64`. Fix by setting the check dtype flag to `False`. Add expected dtypes to the tests fixture and assert that the computation which does the attribute lookup returns the correct type.

`discover` was returning a bytes dtype for the `sa.TIMESTAMP` instead of `datetime_`. That bug was because the SQLAlchemy dialect for MSSQL currently directly imports `sa.TIMESTAMP`, causing a collision in `odo.backends.sql.revtypes`. Fix by 1) creating a subclass of `mssql.TIMESTAMP` to use as the key in `revtypes` so that it does not overwrite `sa.TIMESTAMP` 2) assign that subclass to the mssql dialect's `'TIMESTAMP'` (using `ischema_names`), so that the subclass will be returned by the type engine instead of `mssql.TIMESTAMP`. The added test for the (`sa.TIMESTAMP`, `datetime_`) without the fix applied would result in this error: ``` a = Bytes(), b = DateTime(tz=None), path = ('.measure', "['ts']", '.ty') kwargs = {'check_dim': True, 'check_record_order': True} @assert_dshape_equal.register(object, object) def _base_case(a, b, path=None, **kwargs): > assert a == b, '%s != %s\n%s' % (a, b, _fmt_path(path)) E AssertionError: bytes != datetime E path: _.measure['value'].ty ``` This patch includes checks for the other types besides `sa.TIMESTAMP` for better protection against the general case of failures when using `revtypes` to map SQLAlchemy types to dtypes. Those extra cases exposed an issue with `sa.Float(precision=24)`. That case is commented out to keep the fix of this patch on the `sa.TIMESTAMP` mapping. (I would have used `pytest.param` to mark it xfail, but pytest for this project needs to be upgraded first.) See: blaze#567 blaze#568 https://bitbucket.org/zzzeek/sqlalchemy/issues/4092/type-problem-with-mssqltimestamp blaze/blaze#1656

`discover` was returning a bytes dtype for the `sa.TIMESTAMP` instead of `datetime_`. That bug was because the SQLAlchemy dialect for MSSQL currently directly imports `sa.TIMESTAMP`, causing a collision in `odo.backends.sql.revtypes`. Fix by 1) creating a subclass of `mssql.TIMESTAMP` to use as the key in `revtypes` so that it does not overwrite `sa.TIMESTAMP` 2) assign that subclass to the mssql dialect's `'TIMESTAMP'` (using `ischema_names`), so that the subclass will be returned by the type engine instead of `mssql.TIMESTAMP`. The added test for the (`sa.TIMESTAMP`, `datetime_`) without the fix applied would result in this error: ``` a = Bytes(), b = DateTime(tz=None), path = ('.measure', "['ts']", '.ty') kwargs = {'check_dim': True, 'check_record_order': True} @assert_dshape_equal.register(object, object) def _base_case(a, b, path=None, **kwargs): > assert a == b, '%s != %s\n%s' % (a, b, _fmt_path(path)) E AssertionError: bytes != datetime E path: _.measure['value'].ty ``` This patch includes checks for the other types besides `sa.TIMESTAMP` for better protection against the general case of failures when using `revtypes` to map SQLAlchemy types to dtypes. Those extra cases exposed an issue with `sa.Float(precision=24)`. That case is commented out to keep the fix of this patch on the `sa.TIMESTAMP` mapping. (I would have used `pytest.param` to mark it xfail, but pytest for this project needs to be upgraded first.) See: #567 #568 https://bitbucket.org/zzzeek/sqlalchemy/issues/4092/type-problem-with-mssqltimestamp blaze/blaze#1656

The `datetime.second` attribute was truncating to second resolution, instead of including subsecond values. i.e. `second` attribute of a datetime `2014-01-01 00:00:00.042` would return `0` instead of `0.042` Fix by changing the type of `datetime.Second` from `int64` to `float64`. Returning a float was the behavior before the `compute_up` from `DateTime` -> `ColumnElement` begain enforcing the specified type via a `CAST`.

msgpack seems to have moved

coveralls · 2017-10-20T20:35:10Z

Changes Unknown when pulling 733e469 on thequackdaddy:test into ** on blaze:master**.

postelrich · 2017-10-20T20:52:03Z

@thequackdaddy @ehebert @llllllllll I'm at a loss for why only 3.4 is failing with that mysql error. I tried setting LOCAL as well as set global local_infile=OK. I also made sure they're all running the latest pymysql, yet it still fails. I'm open to suggestions.

Since we're adding 3.6, any objection to removing testing for 3.4? I see a lot of projects don't make builds for 3.4.

llllllllll · 2017-10-20T20:55:42Z

We are still using 3.4 for our blaze servers at Quantopian. We don't use mysql so I would be fine with skipping those tests on 3.4 while we migrate to 3.6.

ehebert · 2017-10-20T21:02:33Z

@llllllllll, I agree, skipping the mysql tests for 3.4 is a good interim solution.

coveralls · 2017-10-20T21:39:48Z