<div dir="ltr">I think I addressed most of these issues in the summary note I jsut posted, but a few specific comments:<br><div class="gmail_extra"><br><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
</span>Unfortunately, I don't think we can change assertAlmostEqual. If we<br>
change the implementation, tests which were passing may fail, and tests<br>
which were failing may pass.<br></blockquote><div><br></div><div>Agreed -- would have thought that was off the table. And ti's really a different test than the proposal -- it is an absolute_tolerance test, but where the tolerance is specified in number of decimal digits after the decimal point (or an optional specific delta) -- not really that useful , but I guess if you have assertTrue(), then why not?<br><br></div>But adding a relative tolerance to unittest makes a lot of sense -- would "assertCloseTo" sound entirely too much like assertAlmostEqual? I think it may be OK if the docs for each pointed to the other.<br><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The actual fuzzy comparison itself is handled by a function<br>
approx_equal(x, y, tol, rel).<br></blockquote><div><br></div><div>NOTE: the difference between this and the my current PEP version is that that absolute tolerance defaults to something other than zero (though it looks like it does default to zero for the assert method), and it is a symmetric test (what Boost calls the "strong" test)<br></div><div><br>- somebody other than me should review NumericTestCase.assertApproxEqual<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
and check that it does nothing unreasonable;<br></blockquote><div><br></div><div>Well it requires the tolerance values to be set on the instance, and they default to zero. So if we were to add this to unittest.TestCase, would you make those instance attributes of TestCase? If so, we want different defaults -- so people could call the assertion with defaults and get something useful. I suggest the one I put in my proposal, naturally ;-)<br><br></div><div> I looked at the underlying function pretty closely. I don't see anything wrong with it. I did a few things differently:<br><br></div><div>- no need to check for NaN explicitly, the comparisons take care of that anyway.<br><br></div><div>- inspired by the boost approach - I used "and" and "or", rather than calling max() -- same result, slightly better performance.<br><br></div><div>But in the end -- essentially the same as the PEP code, except where it's intended to be different.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">- 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, people who disagree with the default definition can simply<br>
subclass TestCase and redefine the approx_equal method.<br>
<br>
(Which is much simpler than having to write the whole assertApproxEqual<br>
method from scratch.)<br></blockquote><div><br></div><div>what assertApproxEqual does is add the ability to test a while sequence of values -- much like numpy's all_close. Do any of the other TestCase assertions provide that?<br><br></div><div>But you could also add an optional parameter to pass in an alternate comparison function, rather than have it be a method of TestCase. As I said, I think it's better to have it available, and discoverable, for use outside of unitest.<br></div><div> <br>Note that in this case, at least, we do want a symmetric version of<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
"is_close", since neither guess nor new_guess is "correct", they are<br>
both approximations.<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>true, but you are also asking the question -- is the new_guess much different that guess. Which points to a asymmetric test -- but either would work.<br><br></div><div>-Chris<br clear="all"></div></div><br>-- <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>