Adding an assertClose() method to unittest.TestCase

[202d193d-f083-461e-a326-928c1d924ea3]

BPO	27198
Nosy	@rhettinger, @gpshead, @mdickinson, @rbtcollins, @ezio-melotti, @bitdancer, @voidspace, @berkerpeksag, @vedgar, @PythonCHB, @iritkatriel
Files	assertClose.patch: patch against recent default branch assertClose.patch: updated with equation in the docs

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-feature', 'library', '3.11'] title = 'Adding an assertClose() method to unittest.TestCase' updated_at = user = 'https://github.com/PythonCHB' ``` bugs.python.org fields: ```python activity = actor = 'ChrisBarker' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'ChrisBarker' dependencies = [] files = ['43157', '43165'] hgrepos = [] issue_num = 27198 keywords = ['patch'] message_count = 23.0 messages = ['267133', '267139', '267150', '267153', '267160', '267168', '267169', '267172', '267176', '267178', '267187', '267215', '267224', '267445', '268878', '268881', '269034', '269036', '269192', '415576', '415586', '415593', '415720'] nosy_count = 12.0 nosy_names = ['rhettinger', 'gregory.p.smith', 'mark.dickinson', 'rbcollins', 'ezio.melotti', 'r.david.murray', 'michael.foord', 'berker.peksag', 'Pam.McANulty', 'veky', 'ChrisBarker', 'iritkatriel'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue27198' versions = ['Python 3.11'] ```

Linked PRs

• gh-96881

[202d193d-f083-461e-a326-928c1d924ea3]

In py3.5, the math.isclose() function was added to the standard library. It can be used to compare floating point numbers to see if they are close to each other, rather than exactly equal. It's not a lot of code, but there are nuances that not every python user should need to understand.

One of the major use cases for isclose() is test code, so it would be good to make it easily available in unitest.TestCase.

TestCase.assertAlmostEqual is NOT the same thing, and can only be used properly for values near 1.

Enclosed is a patch that adds assertClose and assertNotClose to unittest, as well as tests and additions to the docs.

Still pending: should this support complex numbers, there is a cmath.isclose(). If so, but switching on type?

Also -- should this be back-ported to 3.5 as well -- math.isclose() was introduced there.

Done in the pyCon sprints.

[bitdancer]

The word description of the meaning of the tolerance attributes told me absolutely nothing. The equation from the math.islcose docs makes it prefectly clear. So I think the formula should be included in the documentation.

assertAlomstEqual has a delta attribute now, which is equivalent to abs_tol. Perhaps instead we should add rel_tol to assertAlmostEqual?

[202d193d-f083-461e-a326-928c1d924ea3]

Thanks,

I'll add the equation to the docstring and docs.

As for adding a rel_tol to assertAlmostEqual -- I think that's a bad idea -- it's a pretty different concept -- overloading the same method would be more confusing than anything else.

in isclose(), abs_tol is mostly there to support comparison to zero. delta, on the other hand, is consistent with assertAlmost eual in the sense that you need to know the order of magnitude of your values apriori.

[202d193d-f083-461e-a326-928c1d924ea3]

updated patch with the equation in the docs.

[rbtcollins]

Thanks for proposing this. I really don't want to add new assertions to unittest, and I certainly don't want to add narrow usage ones like this, nor ones that are confusingly named (this has nothing to do with files, but 'close' is a verb for files, just like equal is a verb for objects.

Instead, I suggest a regular function that will raise AssertionError on failure. The only thing you need _formatMessage for is handling long messages, and I don't think thats useful or relevant for a binary numeric test like this.

[202d193d-f083-461e-a326-928c1d924ea3]

I'm not sure it's confusing --what would "close" mean for an assertion for a file? "assertClosed" would be confusing -- and an even more trivial assert :-). But we can bikeshed the name if we decide to put this in.

""" I certainly don't want to add narrow usage ones like this """

well, I can't say I like the unittest API either -- but "this would be just as well served with a function" applies to almost all the asserts in TestCase -- assertTrue? really? So unless we're going to deprecate that API, or at least document that you should be using simpler constructs:

assert something

rather than:

self.assertTrue(something)

then we might as well go with it and have a well supported API.

""" I suggest a regular function that will raise AssertionError on failure"""

where would that go? and what would it be called? and how would it be any better than:

assert math.isclose(a,b)

??

The primary reason I think it should be there is discoverability. Folks will be looking through the testCase docs looking for the asserts they need. And they will find assertAlmostEqual -- and it will often be the wrong solution to their problem. Having the right solution be a first-class part of the API is "a good thing"

And it does add a bit of useful information to the message -- knowing the tolerances used for the failing test really helps you know if it's a bug or too strict a test.

[bitdancer]

On the name, I assumed it had something to do with files when I saw the issue title. I'm not sure that's enough of an argument against it, though.

There's an issue where Serhiy is proposing a mixin with additional assert methods. If that has any traction then this could possibly go there.

[202d193d-f083-461e-a326-928c1d924ea3]

thanks, that's bpo-27152 if anyone's curious.

Though I have no idea why you'd want it in a mixin, rather than just there.

But sure, this could be "bundled" in with that.

Perhaps it's time for a broader discussion / consensus about the future of the unittest API?

[bitdancer]

I raised the same point on that issue, and the answer as I understand it is that we want to change the API by making the assert methods independent of the rest of unit test, so I think Serhiy was seeing the mixin as a move in that general direction.

[202d193d-f083-461e-a326-928c1d924ea3]

Would that make folks more amenable to adding more "specialized" asserts? If so, then sure.

I don't know that it takes a PEP (I hope not) but it would be good to have some guidance as to the direction we want unittest to take written down somewhere.

[rbtcollins]

Future direction: hamcrest style matchers. You can read more about them in the context of unittest https://rbtcollins.wordpress.com/2010/05/10/maintainable-pyunit-test-suites/ and http://testtools.readthedocs.io/en/latest/for-test-authors.html#matchers - sorry about the formatting in the blog post, wordpress changed theme details some time after I wrote the post and it now renders horribly :(.

w.r.t. error messages, a regular function that raises AssertionError with a nice message will be precisely as usable.

def assert_math_isclose(first, second, rel_tol=1e-09, abs_tol=0.0, msg=None):
    if math.isclose(first, second, rel_tol=rel_tol, abs_tol=abs_tol):
        return
    standardMsg = ('%s != %s with relative tolerance of %.3g'
                   ' and absolute tolerance of %.3g' % (safe_repr(first),
                                                        safe_repr(second),
                                                        rel_tol,
                                                        abs_tol))
    if msg:
        raise AssertionError("{} : {}".format(standardMsg, msg))
    else:
        raise AssertionError(standardMsg)

The reason I'm pushing back on adding methods to TestCase is that every one we add collides with some number of subclasses out in the wild: the TestCase class has multiple distinct APIs in the same namespace - and thats very poor for maintaining compatibility.

Long term I want to have entirely separate interfaces for these things, but thats even more radical an overhaul than adding matchers as a stock facility, and I don't currently have a planned timeline for starting on that.

All of that said, I see that this isn't the same as assertAlmostEqual in mathematical terms - but in /user/ terms I think it is. So why can't we make assertAlmostEqual be this? Just add the extra parameter needed with a default value and change the implementation?

[202d193d-f083-461e-a326-928c1d924ea3]

"""w.r.t. error messages, a regular function that raises AssertionError with a nice message will be precisely as usable."""

sure -- I totally agree -- but that's not the current unittest API :-( where would you put it? How would people find it and know to use it?

"""The reason I'm pushing back on adding methods to TestCase is that every one we add collides with some number of subclasses out in the wild"""

yup -- but again, the existing API :-(

""" Long term I want to have entirely separate interfaces for these things, but thats even more radical an overhaul than adding matchers as a stock facility, and I don't currently have a planned timeline for starting on that. """ that i like -- but yeah, not this week :-). I also think the matchers thing is a nice idea, but still pretty heavy-weight, and it'll be along time before casual users and newbies will latch onto it..

""" So why can't we make assertAlmostEqual be this? Just add the extra parameter needed with a default value and change the implementation? """

That almost sound like your suggesting we actually change the assertAlmostEqual implementation -- that could be disastrous. On the other hand, you could add a relative_tolerance parameter, and if the user sets that, they are indicating that they want a relative test.

I don't like that API: setting different optional parameters to select an algorithm, when you really just want a different function. On the other hand, it does keep us from polluting the namespace, and allows the documentation for them all to be in the same place, increasing the likelihood that it will be found when it should be.

And if you ask me -- assertAlmostEqual should have had an algorithm like isclose() in the first place ;-)

It will present an additional documentation challenge. (and a bit of ugly switching code.

Now to find some time to do it -- my sprinting time is done :-(

[rhettinger]

FWIW, I find assertClose easy to misinterpret. At first, it looks like an assertion that a file is closed. And once you mentally pronounce it like "close something that is open", it is harder to make the jump to "close as in nearby".

Also, I feed there is very little need for this. that we don't need another way to do it, and that the menagerie of specific asserts is getting harder to learn and remember (i.e. is easier to make up new assert methods than it is to remember what someone else wanted to name it).

[202d193d-f083-461e-a326-928c1d924ea3]

Thanks Raymond.

Damn! I wrote a nice comprehensive note, and my browser lost it somehow :-(.

Here's a shorter version:

"FWIW, I find assertClose easy to misinterpret. At first, it looks like an assertion that a file is closed."

sure -- we can find a new name -- I only used that because it's called "isclose" in the math module -- but no one's likely to think a math module function is about files...

"we don't need another way to do it"

Yes, we certainly do -- it was added to the math module (even though a large fraction of use cases would be testing), and something like this is in numpy, Boost, etc, and as the long debate about the PEP indicates -- it's not obvious how to do it -- I'd argue this is more necessary in unitest than most of the other asserts....

(and, sample size 1: a relatively new user offered to proofread the patch for me, and immediately said "hey this is great -- I've been needing this!"

Frankly, the objections to adding new aseert methods are really a critique of the unittest API -- it is clearly DESIGNED to have specialized asserts for many common use cases. So I think we should either:

Embrace the API and add useful asserts like this one.

or

Make a concerted effort (Primarily through documentation) to move toward a different API (or a different way to use the current one, anyway). Robert's posts about "matches" are a good start.

In the meantime, maybe the way to go with this is to add it to assertAlmostEqual -- it gives folks the functionality, makes it discoverable, and doesn't add a new name.

Any objections to that before I take the time to code that up?

-Chris

[rhettinger]

Embrace the API and add useful asserts like this one.

This kind of philosophy is going to lead to egregious API expansion, making Python harder to learn and remember. You're suggesting that we have a nearly zero resistance to adding new assert variants.

Please keep in mind that once something is added to the standard library, it is very painful to remove it later. As far as I can tell, there has been zero usability testing of this method with actual users and there have been no user requests for it ever. I don't see any analog for it in the unittest modules for other languages.

I don't like the method name at all and think we will regret this method if later an assertIsClosed() method is added for making sure objects have had a close() method called.

If recall correctly, Kent Beck himself opposed this kind of expansion of unittest modules, "Some of the implementations have gotten a little complicated for my taste." He believed that the tool itself should be minimal so that it could be easily learned and mastered while letting "tests be expressed in ordinary source code".

it is clearly DESIGNED to have specialized asserts for many common use cases.

Actually, is wasn't at all. When unittest was added, there were no specialized asserts (see https://docs.python.org/2.1/lib/testcase-objects.html ). It was a translation of the successful and well respected JUnit module. Its design goal was to provide user extendability rather than throwing in everything including the kitchen sink.

Python's unittest module was around for a very long time before the zoo of specialized asserts was added (courtesy of code donated by Google).

[berkerpeksag]

I agree with Raymond. I doubt most of the unittest users would ever need a builtin assertClose() method in unittest. I suggest closing this as 'rejected'.

[202d193d-f083-461e-a326-928c1d924ea3]

Did my comments not get posted, or are they not being read? Anyway:

Could we keep the issues separate here?

1) If you don't like the name, propose another name -- no none has defended this name since an objection was first raised. I"m sure we can find one that would work, if we want to add it.

2) regardless of the original intent, the current API:

• requires fairly substantial effort to write a new assertion, whether as a method or a function. Thus it's a good idea to provide generally useful ones
• has a bunch of assert methods, many of which are no more commonly useful than the proposed method.

So adding this is very much in keeping with the current API.

However, it seems there is much resistance to adding new asserts to the base TestCase. Fine. I have trouble defending that as I don't like the API anyway (and yes, of course, it comes from Java -- that's actually the source of the problem :-) )

But: this is a generally useful and non-trivial assert (indeed, as way too many people are confused about floating point, I think it's pretty critical to provide appropriate tools -- and assertAlmostEqual is NOT the appropriate tool in many cases.

So how to provide this? Options:

1) Robert suggested a stand alone function -- sure, but where to put it?

2) Apparently there is a movement afoot to add a mixin with extra stuff -- sure, that would be fine, but only if there actually is such a mixin.

3) add the functionality to assertAlmostEqual, with a new optional parameter -- I suggested this a few comments back, and have had no feedback. (that at least avoids bike shedding the name...)

My key points:

1) This is generally useful and non-trivial -- so would be good to add to the stdlib.

2) it should be discoverable -- as pointed out, folks have not been clamoring for it -- which means they won't know to go digging for it -- they should find it easily (as many folks will use assertAlmostEqual, when they would be better served by this)

which leaves us with adding to a library of functions or a mixin, only if such a thing will actually be build and documented, with more than one function it it :-)

or adding to assertAlmostEqual.

Feedback on this?

(note: adding assertClose as is now off the table, no more need to argue about that)

[rbtcollins]

Chris, I suggested altering assertAlmostEqual in http://bugs.python.org/issue27198#msg267187 :) - I took your agreement with that as a good thing and didn't reply because I had nothing more to add.

IMO the status of this issue is as you indicated: you needed time to code up the changes, so that its an extension to assertAlmostEquals, rather than a new assertion.

When thats done I'll happily review it and commit it.

[202d193d-f083-461e-a326-928c1d924ea3]

Thanks Robert.

I'll try to find time to re-do the patch soon.

There was enough resistance to the whole idea that I wanted some confirmation that is was worth my time to do that!

Stay tuned.

[iritkatriel]

To summarize the discussion:

There were objections to adding assertClose, but more agreement for adding an option to assertAlomstEqual that does the equivalent.

Chris was to come back with an implementation (that was in 2016).

Are we still pursuing this or shall we close the issue?

[gpshead]

I agree with the decision, assertAlmostEqual is where the feature belongs.

From a practical point of view I suspect a lot of people who want this in the wider world today use just assert math.isclose(...) despite the less useful error message.

[fe5a23f9-4d47-49f8-9fb5-d6fbad5d9e38]

An important point nobody made, as far as I can see:

• the main usability improvement justifying math.isclose is that you don't know the order of magnitude of your correct value--it could be anything (even infinite), and the manner of comparison depends on it. That's why it uses a sensible rel_tol out of the box, but no abs_tol--obviously, since it doesn't want to give nonsense results for e.g. values close to zero.

• but when you write tests, you always know the exact value you should get, right? In almost all cases the second argument is a numeric literal. So the delta-approach is perfectly ok in that context, since you're in control of how much discrepancy you're going to tolerate, and in the moment that you're deciding on this, you have the exact value expected right in front of you.

[202d193d-f083-461e-a326-928c1d924ea3]

Yes -- it was on me years ago to do this.

Honestly, I haven't done it yet because I lost the momentum of PyCon, and I don't personally use unittest at all anyway.

But I still think it's a good idea, and I'd like to keep it open with the understanding that if I don't get it done soon, it'll be closed (or someone else is welcome to do it, of course, if they want)

is, say, three weeks soon enough?

@Vedran Čačić wrote: "... and in the moment that you're deciding on this, you have the exact value expected right in front of you."

Well, yes and no. First of all, not always a literal, though yes, most often it is. But:

1) If the "correct" value is, e.g. 1.2345678e23 -- the delta is not exactly "right there in front of you" -- yes, not that hard to figure out, but it takes a bit of thought, compared to "I want it to be close to this number within about 6 decimal places" (rel_tol=1e-6)

2) Sometimes you have a bunch of values that you are looping over in your tests, or doing parameterized tests -- it which case the relative tolerance could be constant, but the delta is not.

3) With that argument, why do we have the "decimal places" tolerance, rather than a delta always?

Anyway, if I didn't consistently use pytest, I'd want this, so I'm happy to get it done.

Thanks for the ping, @Irit Katriel

[wehlgrundspitze]

There seems again no progress since march. If that is fine for all, I would take the chance to implement the keyword in assertAlmostEqual, since I am also very interested to use it.

[wehlgrundspitze]

During implementing the keyword, I noticed two other closely related issues. I noted them as issue #96882 and a idea: https://discuss.python.org/t/distribution-of-tests-for-unittest-methods-assertxxx/19192. Maybe the people following this issue here can comment on them.

[mdickinson]

I'm not convinced that the proposed assertAlmostEqual addition would be useful, but if it's added, I think the semantics of assertAlmostEqual(val1, val2, delta=d, rel_delta=r) should match those of math.isclose(val1, val2, rel_tol=r, abs_tol=d). Those semantics were hashed out at great length in the PEP 485 discussion, and I think it would be confusing to have assertAlmostEqual doing something different (as #96881 currently does).