TST: better assertion messages on test failures (GH4397)

[jreback]

closes #4397

a start at least

[jreback]

@cpcloud
cc @jtratner

good?

[cpcloud]

what about a name attr check for indices?

[cpcloud]

and maybe just pep8 this sucker while ur at it ...

[jreback]

I think that's handled separately (needs check_names to be true), maybe after @jtratner PR we can turn it on in full

[cpcloud]

oh ok

[cpcloud]

oh i see it's there nvm

[jreback]

peped

[cpcloud]

nice llooks good glad to have some more info here!

[jtratner]

I don't think this is as useful as what we had previously. It needs to raise when one of the components does not have an attribute (e.g., it's the wrong type altogether or something weird has happened). Further, if you do hasattr or getattr, it's not exactly equivalent to trying to get the attribute and defaulting to None otherwise. (and also I think getattr ignores any exception that occurs when trying to get it). So, I think it's better to add an assert_equal that takes an argument which explains. e.g.

assert_equal(left.column.dtype, right.column.dtype, "column dtypes did not match')

Where assert_equal is:

def assert_equal(actual, expected, msg=""):
     assert expected == actual, "%s: %r != %r" % (msg, actual, expected)

Plus, this means that the Exception that will bubbles up if something goes wrong will not be on assert_attr_equal but will instead be on the line assert_equal(left.column.dtype, right.column.dtype, msg) and you'll get the nice error message about not having an attribute and you'll see it's on the column...much easier to debug.

To illustrate the difference, check out this small example:

class SomeClass(object):
    @property
    def failing_property(self):
        return "wefawef".unreal_method(["a"])

obj = SomeClass()
print getattr(obj, "failing_property", None) # no error
print obj.failing_property # fails appropriately

And if you just change the getattr(left, attr, None) to getattr(left, attr), you still lose the nicer error output because the actual error line will read getattr(left, attr) rather than left.column.dtype

[jreback]

you are prob right....feel free to put this in....

my main reason for having this is the problem of

assertion failed....left.dtype == right.dtype...but have no idea what happened