[issue24270] PEP 485 (math.isclose) implementation

Changes by Berker Peksag : -- resolution: -> fixed stage: -> resolved status: open -> closed ___ Python tracker ___ ___ Python-bugs-

[issue24270] PEP 485 (math.isclose) implementation

Tal Einat added the comment: Indeed, it should be. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://m

[issue24270] PEP 485 (math.isclose) implementation

Yury Selivanov added the comment: Can this issue be closed now? -- nosy: +yselivanov ___ Python tracker ___ ___ Python-bugs-list maili

[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 (issu

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

[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

[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

[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 ___ ___ Pyt

[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 ahe

[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 Dec

[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

[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. -- _

[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 commi

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

[issue24270] PEP 485 (math.isclose) implementation

Tal Einat added the comment: Alright then, but is anyone going to review this so that it can go in? The actual code to review is very short, the documentation is clearly written and not too long, and the tests are easy to read! -- ___ Python tracker

[issue24270] PEP 485 (math.isclose) implementation

Stefan Behnel added the comment: > Properly handling other types which are convertible to floats, such as > Decimal and Fraction, is outside the scope of this issue. ... and outside of the scope of the math module in general. It's inherently floating point based. -- __

[issue24270] PEP 485 (math.isclose) implementation

Tal Einat added the comment: @Stefan: Well, this seems to already be the situation with the rest of the math module: >>> math.isinf(Decimal("1e999")) True >>> math.sqrt(Decimal("1e999")) inf Properly handling other types which are convertible to floats, such as Decimal and Fraction, is outsi

[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 ___

[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 __

[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 i

[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 TestCa

[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

[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 separ

[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 cmat

[issue24270] PEP 485 (math.isclose) implementation

Changes by Tal Einat : -- nosy: +mark.dickinson, rhettinger, stutzbach ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubs

[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

[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 th

[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

[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

[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 ___ ___

[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/Pyt