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

Convert use of assert_/assert to specialized asserts in tests #6175

Closed
ghost opened this issue Jan 29, 2014 · 11 comments
Closed

Convert use of assert_/assert to specialized asserts in tests #6175

ghost opened this issue Jan 29, 2014 · 11 comments
Labels
Testing pandas testing functions or related to the test suite
Milestone

Comments

@ghost
Copy link

ghost commented Jan 29, 2014

An example of an exception that uses assert_:

============================================================================
FAILURE: test_class_ops (pandas.tseries.tests.test_timeseries.TestTimestamp)
----------------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\Python33-AMD64\Lib\unittest\case.py", line 384, in _executeTestPart
    function()
  File "C:\workspace\pandas_tests\BITS\64\PYTHONVER\33\pandas\tseries\tests\test_timeseries.py", line 2513, in test_class_ops
    compare(Timestamp.now('UTC'),datetime.now(pytz.timezone('UTC')))
  File "C:\workspace\pandas_tests\BITS\64\PYTHONVER\33\pandas\tseries\tests\test_timeseries.py", line 2510, in compare
    self.assert_(int(Timestamp(x).value/1e9) == int(Timestamp(y).value/1e9))
  File "c:\Python33-AMD64\Lib\unittest\case.py", line 1140, in deprecated_func
    return original_func(*args, **kwargs)
  File "c:\Python33-AMD64\Lib\unittest\case.py", line 520, in assertTrue
    raise self.failureException(msg)
AssertionError: False is not true

note the line:

self.assert_(int(Timestamp(x).value/1e9) == int(Timestamp(y).value/1e9))

if this had been assertEqual, we'd have seen what the value were
and been able to debug easier (now fixed). (though, It should be obvious why that test is unreliable).

There are lots of these across the tests.
If someone steps up to do this, do a little bit at a time.
a PR that touches 200 tests will defy review and be stuck forever
in PR limbo.

@jreback
Copy link
Contributor

jreback commented Jan 29, 2014

anyway just to fix assert_ itself?
I use that almost exclusively

@ghost
Copy link
Author

ghost commented Jan 29, 2014

Using self.assert_(x == y) doesn't tell you what x and y were when failing,
assertEqual does, so at least in that case you should use assertEqual.

@jreback
Copy link
Contributor

jreback commented Jan 29, 2014

and you learn something new every day...ok!

@ghost
Copy link
Author

ghost commented Jan 29, 2014

On a good day, I do.

@ghost
Copy link
Author

ghost commented Feb 4, 2014

Similar problem from test_parsing_timezone_offsets and co'

        self.assert_(
            np.array_equal(
                tslib.array_to_datetime(arr),
                np.array(
                    [
                        '2013-01-01T00:00:00.000000000-0000',
                        '2013-01-02T00:00:00.000000000-0000'
                    ],
                    dtype='M8[ns]'
                )
            )
        )

which gives an uniformative

AssertionError: False is not true

@bwignall, care to take another step? We could use a more informative
version of np.array_equal to added to pd.util.testing, borrowing from
the assert_series_equal function there perhaps, and a refactor to go with it.

@bwignall
Copy link
Contributor

bwignall commented Feb 5, 2014

I will chip away at this more, @y-p. I was thinking of continuing on the assert_(x == y) theme, then circling back to other forms.

@hayd
Copy link
Contributor

hayd commented Mar 9, 2014

Just to point out, some of these aren't supported in 2.6 e.g. assertIsInstance, should we worry about that ? travis hasn't been able to test on 2.6 for a good while...

👍 on changing them all.

Note: they are backported in unittest2 - so we could import unittest2 as unittest in util/testing for py < 27?...

@jreback
Copy link
Contributor

jreback commented Mar 9, 2014

@hayd what do u mean
2.6 build tests

@jreback
Copy link
Contributor

jreback commented Mar 9, 2014

@bwignall leave this issue open?

@bwignall
Copy link
Contributor

bwignall commented Mar 9, 2014

@hayd: tm.TestCase has versions of these, which should be API-compatible with the Python 2.7+ functions.

@jreback: I just went through the assertions again, and with the latest (#6580) PR, it looks like all of the straightforward changes have been made. There are still some assert_(...) statements, but those could all be changed to assertTrue(...) or assertFalse(...) at this point, if desired.

@jreback jreback closed this as completed Mar 9, 2014
@jreback
Copy link
Contributor

jreback commented Mar 9, 2014

@bwignall that's for all the effort in this!
closing this issue but if u see things that need change pls submit additional PRs

gouthambs pushed a commit to gouthambs/pandas that referenced this issue Mar 12, 2014
Work on pandas-dev#6175. Changes instances of assert_(a [not] in b) to specialized assert[Not]In(a,b).
gouthambs pushed a commit to gouthambs/pandas that referenced this issue Mar 12, 2014
Work on pandas-dev#6175. Changes instances of assert_(a is [not] None) to specialized assertIs[Not]None(a).
gouthambs pushed a commit to gouthambs/pandas that referenced this issue Mar 12, 2014
Work on pandas-dev#6175. This is the third (and final) "half" of the work started in pandas-dev#6368.
gouthambs pushed a commit to gouthambs/pandas that referenced this issue Mar 12, 2014
Work on pandas-dev#6175. Changes instances of assert_(a is [not] b) to specialized assertIs[Not](a, b).
gouthambs pushed a commit to gouthambs/pandas that referenced this issue Mar 12, 2014
Work on pandas-dev#6175. Changes instances of assert_(a [not] in b) to specialized assert[Not]In(a, b).
gouthambs pushed a commit to gouthambs/pandas that referenced this issue Mar 12, 2014
Work on pandas-dev#6175. Changes instances of assert_([not] isinstance(a,b)) to specialized assert[Not]IsInstance(a, b).
gouthambs pushed a commit to gouthambs/pandas that referenced this issue Mar 12, 2014
Work on pandas-dev#6175. Change superclass of tests/test_compat, to use tm.TestCase
gouthambs pushed a commit to gouthambs/pandas that referenced this issue Mar 12, 2014
Finishes pandas-dev#6175, to the extent that everything remaining can be mapped to assertTrue or assertFalse, unless larger refactorings are desired.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testing pandas testing functions or related to the test suite
Projects
None yet
Development

No branches or pull requests

3 participants