-
-
Notifications
You must be signed in to change notification settings - Fork 32k
bpo-29962: add math.remainder #950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Doc/library/math.rst
Outdated
@@ -175,6 +175,27 @@ Number-theoretic and representation functions | |||
of *x* and are floats. | |||
|
|||
|
|||
.. function:: remainder(x, y) | |||
|
|||
Return the IEEE 754-style remainder of ``x`` with respect to ``y``. For |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a bit of inconsistency in the file, but I see that in general the parameters do not have a code markup but italic: x and y instead of x
and y
.
I also see in general there are two whitespaces after the period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Fix on the way.
Doc/library/math.rst
Outdated
@@ -175,6 +175,27 @@ Number-theoretic and representation functions | |||
of *x* and are floats. | |||
|
|||
|
|||
.. function:: remainder(x, y) | |||
|
|||
Return the IEEE 754-style remainder of ``x`` with respect to ``y``. For |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameters usually are formatted with italic. *x*
and *y*
.
@@ -175,6 +175,27 @@ Number-theoretic and representation functions | |||
of *x* and are floats. | |||
|
|||
|
|||
.. function:: remainder(x, y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be named remainder_near
as decimal.remainder_near
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not clear to me: see discussion in the bpo issue.
Doc/library/math.rst
Outdated
Return the IEEE 754-style remainder of ``x`` with respect to ``y``. For | ||
finite ``x`` and finite nonzero ``y``, this is the difference ``x - n*y``, | ||
where ``n`` is the closest integer to the exact value of the quotient ``x / | ||
y``. If ``x / y`` is exactly halfway between two consecutive integers, the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentences are separated by two spaces in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I think I caught all the single spaces.
Doc/library/math.rst
Outdated
nearest *even* integer is used for ``n``. The remainder ``r = remainder(x, | ||
y)`` thus always satisfies ``abs(r) <= 0.5 * abs(y)``. | ||
|
||
Special cases follow IEEE 754: in particular, ``remainder(x, math.inf)`` is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And remainder(x, -math.inf)
?
Doc/whatsnew/3.7.rst
Outdated
math | ||
---- | ||
|
||
New ``remainder`` function, implementing the IEEE 754-style remainder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make it a reference? :func:'~math.remainder'
?
Modules/mathmodule.c
Outdated
} | ||
else { | ||
assert(m == c); | ||
r = m - 2.0 * fmod(0.5 * (absx - m), absy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add any comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
The coverage complaint seems to be because the line
in the tests isn't being covered, and that's because none of the tests are failing. I'm not sure what to do about that. |
Doc/whatsnew/3.7.rst
Outdated
@@ -111,6 +111,12 @@ Serhiy Storchaka in :issue:`28682`.) | |||
Added support for :ref:`file descriptors <path_fd>` in :func:`~os.scandir` | |||
on Unix. (Contributed by Serhiy Storchaka in :issue:`25996`.) | |||
|
|||
math |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
math
should be between locale
and os
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, so it should. Thanks!
…pite of possible rounding error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small questions about the tests
# triples (x, y, remainder(x, y)) in hexadecimal form. | ||
testcases = [ | ||
# Remainders modulo 1, showing the ties-to-even behaviour. | ||
'-4.0 1 -0.0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for using strings to be splitted instead of a more natural container like tuples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just personal preference, really. I find the individual cases to be easier to read and write this way. Comparing:
'0x1.921fb54442d18p+0 0x1.921fb54442d18p+2 0x1.921fb54442d18p+0',
'-1.4 -0.c 0.4',
with
('0x1.921fb54442d18p+0', '0x1.921fb54442d18p+2', '0x1.921fb54442d18p+0'),
('-1.4', '-0.c', '0.4'),
there's a lot more noise in the latter representation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, if Python had direct support for hex floating-point literals, then
(0x1.921fb54442d18p+0, 0x1.921fb54442d18p+2, 0x1.921fb54442d18p+0),
would be the obvious representation. But it doesn't. :-(
actual = math.remainder(-x, y) | ||
validate_spec(-x, y, actual) | ||
|
||
# Special values. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these cases be moved into tests on their own?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly. I wanted to stay (somewhat) consistent with the existing test structure, which is mostly one test method per wrapped libm function. One day it would be nice to rework test_math
. But not today.
Add IEEE 754-style remainder operation.