Skip to content

bpo-26680: Incorporate is_integer in all built-in and standard library numeric types #6121

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.

Sign up for GitHub

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

Merged
mdickinson merged 10 commits into from
Oct 1, 2020
Merged

bpo-26680: Incorporate is_integer in all built-in and standard library numeric types #6121

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d80b017
bpo-26680: Adds support for int.is_integer() for compatibility with f…
rob-smallshire Mar 13, 2018
b46a9dc
bpo-26680: Adds a test to ensure that False.is_integer() and True.is_…
rob-smallshire Mar 13, 2018
3b1677f
bpo-26680: Adds Real.is_integer() with a trivial implementation using…
rob-smallshire Mar 13, 2018
f78bf5c
bpo-26680: Adds Rational.is_integer which returns True if the denomin…
rob-smallshire Mar 13, 2018
6a905a8
bpo-26680: Adds Integral.is_integer which always returns True.
rob-smallshire Mar 13, 2018
cc07ee5
bpo-26680: Adds tests for Fraction.is_integer called as an instance m…
rob-smallshire Mar 13, 2018
3ae9866
bpo-26680: Updates documentation for Real.is_integer and built-ins in…
rob-smallshire Mar 14, 2018
54ca820
bpo-26680: Adds Decimal.is_integer to the Python and C implementations.
rob-smallshire Mar 14, 2018
a9e364d
bpo-26680: Updates the ACKS file.
rob-smallshire Mar 15, 2018
11db7f8
bpo-26680: NEWS entries for int, the numeric ABCs and Decimal.
rob-smallshire Mar 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
bpo-26680: Adds Rational.is_integer which returns True if the denomin… 
…ator is one.

This implementation assumes the Rational is represented in it's
lowest form, as required by the class docstring.
  • Loading branch information
@rob-smallshire
rob-smallshire committed Sep 18, 2020
commit f78bf5c9d726801e67ff1bd8d13797d77d4967b6
4 changes: 4 additions & 0 deletions Lib/numbers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@ def __float__(self):
"""
return self.numerator / self.denominator

def is_integer(self):
"""Return True if the Rational is integral; otherwise return False."""
return self.denominator == 1
Copy link
Copy Markdown
Contributor

@eric-wieser eric-wieser Sep 18, 2020

Choose a reason for hiding this comment

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

Must denominator and numerator always be fully reduced?

Copy link
Copy Markdown
Contributor

@eric-wieser eric-wieser Sep 18, 2020

Choose a reason for hiding this comment

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

If not,

Suggested change
return self.denominator == 1
return self.numerator % self.denominator == 0

Copy link
Copy Markdown
Member

@mdickinson mdickinson Sep 18, 2020

Choose a reason for hiding this comment

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

Yes, self.numerator and self.denominator will always be relatively prime in normal use (and denominator will always be positive). Other parts of the fractions module assume this, so it's safe to just check self.denominator == 1 here.

Copy link
Copy Markdown
Contributor

@eric-wieser eric-wieser Sep 18, 2020

Choose a reason for hiding this comment

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

This isn't about Fraction and its implementation of the Rational API though - this is about whether the API mandates the implementation behaves this way.

Copy link
Copy Markdown
Contributor

@eric-wieser eric-wieser Sep 18, 2020

Choose a reason for hiding this comment

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

Ah. I see the doc is """.numerator and .denominator should be in lowest terms.""" - so perhaps worth clarifying that denominator should always be positive too.

Copy link
Copy Markdown
Member

@mdickinson mdickinson Sep 19, 2020

Choose a reason for hiding this comment

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

Yes, that docstring could definitely be improved. Apart from that clarification, we could do with a line or two describing what the Rational class is actually for, rather than launching straight into a detail. But that's a job for a separate PR.



class Integral(Rational):
"""Integral adds a conversion to int and the bit-string operations."""
Expand Down
16 changes: 15 additions & 1 deletion Lib/test/test_numeric_tower.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import sys
import operator

from numbers import Real
from numbers import Real, Rational, Integral
from decimal import Decimal as D
from fractions import Fraction as F

Expand Down Expand Up @@ -198,6 +198,7 @@ def test_complex(self):
self.assertRaises(TypeError, op, z, v)
self.assertRaises(TypeError, op, v, z)


class IsIntegerTest(unittest.TestCase):

def test_real_is_integer(self):
Expand All @@ -209,5 +210,18 @@ def test_real_is_integer(self):
self.assertFalse(Real.is_integer(-0.5))
self.assertFalse(Real.is_integer(4.2))

def test_rational_is_integer(self):
self.assertTrue(Rational.is_integer(F(-1, 1)))
self.assertTrue(Rational.is_integer(F(0, 1)))
self.assertTrue(Rational.is_integer(F(1, 1)))
self.assertTrue(Rational.is_integer(F(42, 1)))
self.assertTrue(Rational.is_integer(F(2, 2)))
self.assertTrue(Rational.is_integer(F(8, 4)))

self.assertFalse(Rational.is_integer(F(1, 2)))
self.assertFalse(Rational.is_integer(F(1, 3)))
self.assertFalse(Rational.is_integer(F(2, 3)))


if __name__ == '__main__':
unittest.main()