[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Indeed, it should be. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Changes by Berker Peksag berker.pek...@gmail.com: -- resolution: - fixed stage: - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Yury Selivanov added the comment: Can this issue be closed now? -- nosy: +yselivanov ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Roundup Robot added the comment: New changeset bbb3a3129c12 by Serhiy Storchaka in branch '3.5': Moved Misc/NEWS entry (issue #24270) to correct section. https://hg.python.org/cpython/rev/bbb3a3129c12 New changeset ff1938d12240 by Serhiy Storchaka in branch 'default': Moved Misc/NEWS entry (issue #24270) to correct section. https://hg.python.org/cpython/rev/ff1938d12240 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: I've just committed this into 3.5 and 3.6. (I accidentally included the wrong issue number in the commit message, so the bot hasn't posted here about it. Sorry!) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Christopher Barker added the comment: I wrote: I will update the PEP to indicate that it is float-only, or complex for the cmath implementation (thanks, Tal!). Done: https://github.com/PythonCHB/close_pep/blob/master/pep-0485.txt Hopefully pushed to the official PEP repo soon. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Stefan Krah added the comment: While we're on the subject, though, trying to convert a very large int to float fails with an OverflowError. Perhaps Decimal should do the same? I've always viewed float() as a cast. For casting Decimal's behavior seems to be the right one to me. If we go ahead with implicit casts for isclose(), I still think the PEP's non-float section should be changed: It sounds as if native support was planned for Decimal. Does someone have the tracker id of Chris? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Christopher Barker added the comment: Sorry for the confusion: when I wrote the PEP, I was thinking in terms of a Python, duck-typed implementation. Now that it's in C, that doesn't work so well. I will update the PEP to indicate that it is float-only, or complex for the cmath implementation (thanks, Tal!). Any other type will be converted to float if possible, with the limitations that that has. As for Decimal -- the right thing to do would be to do a native Decimal implementation -- but that would live in the decimal module, maybe as a method on the Decimal type. (or stand alone -- not sure) Fraction doesn't have the same precision issues as floats, so not sure what the point is. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: It's Chris.Barker. I've added him to the nosy list. -- nosy: +Chris.Barker ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: @Stefan K.: I tend to agree, but still think that's a separate issue. math.isclose() certainly shouldn't be checking the type of its arguments. While we're on the subject, though, trying to convert a very large int to float fails with an OverflowError. Perhaps Decimal should do the same? float(10**999) Traceback (most recent call last): File stdin, line 1, in module OverflowError: int too large to convert to float math.isclose(10**999, 10**40) Traceback (most recent call last): File stdin, line 1, in module OverflowError: int too large to convert to float -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Hopefully final patch attached. This adds cmath.isclose() along with relevant tests and documentation. Note that cmath.isclose() rejects complex tolerances -- only the values may be complex. -- Added file: http://bugs.python.org/file39532/isclose.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Stefan Krah added the comment: I think users may be surprised that any two large Decimals like 1e40 and 1e999 are close. In the Decimal world these aren't infinite. -- nosy: +skrah ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Paul Moore added the comment: Looks OK to me. I assume the differences between the math and cmath code and tests is because cmath uses Argument Clinic and math doesn't, and cmath uses unittest.main whereas math adds the suites manually? As far as I can see, that's what's going on. -- nosy: +paul.moore ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Indeed, those are major reasons for differences. I avoided using Argument Clinic for math.isclose() because there is a pending conversion patch for the entire math module and I didn't want to cause unnecessary merge conflicts. Is Paul's okay enough for me to commit this, or should we also get an okay from one of the three people listed next to the math module on the experts index? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Stefan Krah added the comment: It's inherently floating point based. Except for floor() and ceil() though. The wording in the PEP under non-float types made me think that something similar was intended here. Personally I'm fine with math being float-only. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Stefan Krah added the comment: Also, I must say that returning inf in sqrt() bothers me much less than the assertion that two numbers with a gigantic relative error have a relerr of 1e-9. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Attached yet another revised version of the math.isclose() patch. This patch fixes a problem with the tests in the previous patch which causes them to fail when the full test suite is run. I've also slightly reworded the doc-string. Hopefully this is ready to go in! -- Added file: http://bugs.python.org/file39518/math_isclose_v4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Attached is a revised patch including changed due to the reviews by Berker and Yuri. The significant changes from the previous patch are: 1. The rel_tol and abs_tol parameters have been made keyword-only. 2. The tests have been extracted into a separate TestCase sub-class. They are now better organized and will be easy to re-use for testing cmath.isclose when it is added (hopefully soon). -- Added file: http://bugs.python.org/file39509/math_isclose_v3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Significant questions brought up by Berker Peksağ in his review of the latest patch (thanks for the review!): 1. Should the tolerance parameters be keyword-only? Berker suggests that they should be. I agree. 2. Should the math.isclose() tests be split into a separate TestCase class with many separate methods? It is currently a single method which does all of the testing for math.isclose(). (Chris's original code has it separated into several TestCase classes; I consolidated it into a single method to keep in line with the current structure of the math module's tests.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Regarding the tests, I now realize that most of them should be reused for testing cmath.isclose(), which means they'll have to be defined outside of test_math.MathTests. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Attached is a slightly revised patch. This mostly fixes minor documentation wording and formatting issues, including those pointed out by Chris Barker on the python-dev mailing list. Also, since it has been decided to support complex values only in a separate cmath.isclose() function, the previously included commented-out complex test cases have been removed. -- Added file: http://bugs.python.org/file39495/math_isclose_v2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Changes by Tal Einat talei...@gmail.com: -- nosy: +mark.dickinson, rhettinger, stutzbach ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Stefan Behnel added the comment: Eventually, I think a corresponding function should be added to cmath. math.isclose() shouldn't deal with complex values by itself (other than rejecting them as non-floatish input). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: Attached is a patch based on Chris Barker's implementation on github[1]. This includes only the C implementation, as well as tests, documentation and entries in NEWS and whatsnew. I had to keep the (PyCFunction) cast in the module method list in Modules/mathmodule.c. That part should certainly be reviewed. As for documentation etc., I took the best wording I could find from the PEP and the docs in the github repo, and made very slight changes only where I though they made things significantly clearer. .. [1]: https://github.com/PythonCHB/close_pep -- keywords: +patch Added file: http://bugs.python.org/file39481/math_isclose.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Stefan Behnel added the comment: The cast is correct and required (the actual signature is determined by the METH_* flags). Patch LGTM, FWIW. -- components: +Library (Lib) nosy: +scoder type: - enhancement ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: I have a question regarding complex values. The code (from Chris Barker) doesn't support complex values (only things that can be converted into doubles). However, the PEP states the following under Non-float types: complex : for complex, the absolute value of the complex values will be used for scaling and comparison. If a complex tolerance is passed in, the absolute value will be used as the tolerance. Should math.isclose() support complex values? Should an equivalent function be added to cmath? Should we just leave things as they are and remove mention of complex values from the PEP (it isn't mentioned in the docs)? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
Tal Einat added the comment: I'm now working this into a patch against current default. -- nosy: +taleinat ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24270] PEP 485 (math.isclose) implementation
New submission from Nick Coghlan: Tracking issue for the PEP 485 math.isclose() implementation: https://www.python.org/dev/peps/pep-0485/ Chris's implementation review request to python-dev: https://mail.python.org/pipermail/python-dev/2015-May/140031.html Working repo: https://github.com/PythonCHB/close_pep -- messages: 243914 nosy: ncoghlan priority: normal severity: normal status: open title: PEP 485 (math.isclose) implementation versions: Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24270 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
