<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Jan 25, 2015 at 10:39 PM, Steven D'Aprano <span dir="ltr"><<a href="mailto:steve@pearwood.info" target="_blank">steve@pearwood.info</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">On Sun, Jan 25, 2015 at 05:21:53PM -0800, Chris Barker wrote:<br><span class="">> But adding a relative tolerance to unittest makes a lot of sense -- would<br>
> "assertCloseTo" sound entirely too much like assertAlmostEqual? I think it<br>
> may be OK if the docs for each pointed to the other.<br>
<br>
</span>CloseTo assumes an asymetric test, which isn't a given :-)<br>
<br>
I prefer ApproxEqual, although given that it is confusingly similar to<br>
AlmostEqual, IsClose would be my second preference.</blockquote><div><br></div><div>indeed -- I did add the "to" to imply the asymmetric test -- so I say if we go with asymmetric test then IsClose and the IsCloseTo if we go with the asymmetric test.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">> Well it requires the tolerance values to be set on the instance, and they<br>
> default to zero. So if we were to add this to unittest.TestCase, would you<br>
> make those instance attributes of TestCase?<br>
<br>
</span>No, I would modify it to do something like this:<br>
<br>
if tol is None:<br>
tol = getattr(self, "tol", 0.0) # or some other default<br>
<br>
and similar for rel.<br></blockquote><div><br></div><div>so the attributes would not be there by default but users could add them if they want:</div><div><br></div><div>class my_tst(unitest.TestCase):</div><div> tol = 1e-8</div><div><br></div><div>That would work, but seems like a pretty unclear API to me -- is there a precedent in unitest for this already?</div><div><br></div><div>But I'll leave further discussion on that to other -- I don't like the UnitTest API anyway ;-)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I recommend using short names for the two error tolerances, tol and rel,<br>
because if people are going to be writing a lot of tests, having to<br>
write:<br>
<br>
self.assertIsClose(x, y, absolute_tolerance=0.001)<br>
<br>
will get tiresome.</blockquote><div><br></div><div>Isn't that why you set an attribute on your class?</div><div><br></div><div>but if short, at least rel_tol and abs_tol a plain "tol" could be too confusing (even though I did that in my first draft...)</div><div><br></div><div>My current draft has rel_tolerance and abs_toelraance -- perhaps a bit too long to type often, but a few people asked for longer, more descriptive names.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> > - since there are considerable disagreements about the right way to<br>
> > handle a fuzzy comparison when *both* an absolute and relative error<br>
> > are given, </span></blockquote><div><br></div><div>Is there? In this discussion , no one had any issue with the proposed approach:</div><div><br></div><div>result = difference <= rel_tolerance*scaling_value or difference <= abs_tolerance</div><div><br></div><div>The only issue brought up is that we might want to do it the numpy way for the sake of compatibility with numpy. That's why I didn't add it to my list of issues to resolve.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">I was motivated by assertEqual and the various sequence/list methods. </blockquote><div><br></div><div>yup -- good to keep that trend going.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">> But you could also add an optional parameter to pass in an alternate<br>
> comparison function, rather than have it be a method of TestCase. As I<br>
> said, I think it's better to have it available, and discoverable, for use<br>
> outside of unitest.<br>
<br>
</span>That's an alternative too. I guess it boils down to whether you prefer<br>
inheritance or the strategy design pattern :-)<br></blockquote><div><br></div><div>Now that I think about it -- we could easily do both.</div><div><br></div><div>Define a math.is_close_to()</div><div><br></div><div>in TestCase:</div><div><br></div><div> @staticmethod</div><div> def is_close_to(*args, **kwargs):</div><div> return math.isclose_to(*args, **kwargs)</div><div><br></div><div>best of both worlds. I"ve got my stand alone function outside unittest, and folks can still override TestCase.is_close_to if they want.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I do think there are two distinct use-cases that should be included in<br>
the PEP:<br>
<br>
(1) Unit testing, and a better alternative to assertAlmostEqual.<br>
<br>
(2) Approximate equality comparisons, as per Guido's example.<br></blockquote><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Note that those two are slightly different: in the unit testing case,<br>
you usually have an known expected value (not necessarily mathematically<br>
exact, but at least known) while in Guido's example neither value is<br>
necessarily better than they other, you just want to stop when they are<br>
close enough.<br></blockquote><div><br></div><div>yes, but in an iterative solution you generally compute a solution, then use that to compute a new solution, and you want to know if the new one is significantly different than the previous -- so an asymmetric test does make some sense. But again either work work, and pretty much the same.</div><div><br></div><div>Example forthcoming....</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Like Nick, I think the first is the more important one. In the second<br>
case, anyone writing a numeric algorithm is probably copying an<br>
algorithm which already incorporates a fuzzy comparison, or they know<br>
enough to write their own. The benefits of a standard solution are<br>
convenience and correctness. Assuming unittest provides a well-tested<br>
is_close/approx_equal function, why not use it?</blockquote><div><br></div><div>Exactly.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">I can see we're going to have to argue about the "Close To" versus<br>
"Close" distinction :-)<br></blockquote><div><br></div><div>I think we both understand and agree on the distinction. My take is:</div><div><br></div><div> - Either will work fine in most instances</div><div> - The asymmetric one is a bit clearer and maybe better for the testing use-case.</div><div> - I'd be perfectly happy with either on in the standard library</div><div><br></div><div>Maybe not consensus, but the majority on this thread seem to prefer the asymmetric test.</div><div><br></div><div>We could, of course add a flag to turn on the symmetric test (probably the Boost "strong" case), but I'd rather not have more flags, and as you indicate above, the people for whom it matters will probably write their own comparison criteria anyway.</div><div><br></div><div>It looks like we need to add a bunch of tet to the PEP about incorporating this into unitest -- I'd love it if someone else wrote that -- I'm not much of a unitest user anyway.</div><div><br></div><div>Pull requests accepted:</div><div><br></div><div><a href="https://github.com/PythonCHB/close_pep">https://github.com/PythonCHB/close_pep</a><br></div><div><br></div><div><br></div><div>-Chris</div></div><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><br>Christopher Barker, Ph.D.<br>Oceanographer<br><br>Emergency Response Division<br>NOAA/NOS/OR&R (206) 526-6959 voice<br>7600 Sand Point Way NE (206) 526-6329 fax<br>Seattle, WA 98115 (206) 526-6317 main reception<br><br><a href="mailto:Chris.Barker@noaa.gov" target="_blank">Chris.Barker@noaa.gov</a></div>
</div></div>