Skip to content

bpo-29962: add math.remainder#950

Merged
mdickinson merged 7 commits intopython:masterfrom
mdickinson:enh/math-remainder
Apr 5, 2017
Merged

bpo-29962: add math.remainder#950
mdickinson merged 7 commits intopython:masterfrom
mdickinson:enh/math-remainder

Conversation

@mdickinson
Copy link
Copy Markdown
Member

Add IEEE 754-style remainder operation.

Comment thread Doc/library/math.rst Outdated

.. function:: remainder(x, y)

Return the IEEE 754-style remainder of ``x`` with respect to ``y``. For
Copy link
Copy Markdown
Contributor

@marco-buttu marco-buttu Apr 1, 2017

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.

Copy link
Copy Markdown
Member Author

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.

Comment thread Doc/library/math.rst Outdated

.. function:: remainder(x, y)

Return the IEEE 754-style remainder of ``x`` with respect to ``y``. For
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Apr 1, 2017

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

Comment thread Doc/library/math.rst
of *x* and are floats.


.. function:: remainder(x, y)
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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.

Comment thread 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
Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member Author

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.

Comment thread Doc/library/math.rst
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
Copy link
Copy Markdown
Member

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)?

Comment thread Doc/whatsnew/3.7.rst Outdated
math
----

New ``remainder`` function, implementing the IEEE 754-style remainder
Copy link
Copy Markdown
Member

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'?

Comment thread Modules/mathmodule.c
}
else {
assert(m == c);
r = m - 2.0 * fmod(0.5 * (absx - m), absy);
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@mdickinson
Copy link
Copy Markdown
Member Author

The coverage complaint seems to be because the line

self.fail("Expected a NaN, got {!r}.".format(value))

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.

Comment thread Doc/whatsnew/3.7.rst Outdated
Added support for :ref:`file descriptors <path_fd>` in :func:`~os.scandir`
on Unix. (Contributed by Serhiy Storchaka in :issue:`25996`.)

math
Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member Author

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!

Copy link
Copy Markdown
Contributor

@SylvainDe SylvainDe left a 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

Comment thread Lib/test/test_math.py
# triples (x, y, remainder(x, y)) in hexadecimal form.
testcases = [
# Remainders modulo 1, showing the ties-to-even behaviour.
'-4.0 1 -0.0',
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Member Author

@mdickinson mdickinson Apr 5, 2017

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.

Copy link
Copy Markdown
Member Author

@mdickinson mdickinson Apr 5, 2017

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. :-(

Comment thread Lib/test/test_math.py
actual = math.remainder(-x, y)
validate_spec(-x, y, actual)

# Special values.
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Member Author

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.

@mdickinson mdickinson merged commit a0ce375 into python:master Apr 5, 2017
@mdickinson mdickinson deleted the enh/math-remainder branch April 5, 2017 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants