[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Vedran Čačić]

Vedran Čačić  added the comment:

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.

--
nosy: +veky

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Gregory P. Smith]

Gregory P. Smith  added the comment:

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.

--
nosy: +gregory.p.smith
status: pending -> open
versions: +Python 3.11 -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Irit Katriel]

Irit Katriel  added the comment:

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?

--
nosy: +iritkatriel
status: open -> pending

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Robert Collins]

Robert Collins added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

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)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Berker Peksag]

Berker Peksag added the comment:

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'.

--
nosy: +berker.peksag

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Raymond Hettinger]

Raymond Hettinger added the comment:

> 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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Pam McA'Nulty]

Changes by Pam McA'Nulty :


--
nosy: +Pam.McANulty

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Raymond Hettinger]

Raymond Hettinger added the comment:

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).

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

"""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 :-(

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Robert Collins]

Robert Collins added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[R. David Murray]

R. David Murray added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

thanks, that's Issue27152 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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[R. David Murray]

R. David Murray added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Robert Collins]

Robert Collins added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

updated patch with the equation in the docs.

--
Added file: http://bugs.python.org/file43165/assertClose.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[R. David Murray]

Changes by R. David Murray :


--
nosy: +mark.dickinson

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

Chris Barker added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[R. David Murray]

R. David Murray added the comment:

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?

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27198] Adding an assertClose() method to unittest.TestCase

[Chris Barker]

New submission from Chris Barker:

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.

--
components: Library (Lib)
files: assertClose.patch
keywords: patch
messages: 267133
nosy: ChrisBarker, ezio.melotti, michael.foord, rbcollins
priority: normal
severity: normal
status: open
title: Adding an assertClose() method to unittest.TestCase
type: enhancement
versions: Python 3.6
Added file: http://bugs.python.org/file43157/assertClose.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com