Skip to content

bpo-36887: add math.isqrt#13244

Merged
mdickinson merged 17 commits intopython:masterfrom
mdickinson:math-isqrt
May 18, 2019
Merged

bpo-36887: add math.isqrt#13244
mdickinson merged 17 commits intopython:masterfrom
mdickinson:math-isqrt

Conversation

@mdickinson
Copy link
Copy Markdown
Member

@mdickinson mdickinson commented May 11, 2019 •
edited by bedevere-bot
Loading

Add an integer square root function to the math module.

https://bugs.python.org/issue36887

mdickinson
mdickinson commented May 11, 2019
View reviewed changes
Comment thread Modules/mathmodule.c Outdated
mdickinson
mdickinson commented May 11, 2019
View reviewed changes
Comment thread Modules/mathmodule.c Outdated
mdickinson
mdickinson commented May 11, 2019
View reviewed changes
Comment thread Modules/mathmodule.c Outdated
mdickinson
mdickinson commented May 11, 2019
View reviewed changes
Comment thread Modules/mathmodule.c Outdated
mdickinson
mdickinson commented May 11, 2019
View reviewed changes
Comment thread Doc/library/math.rst Outdated
serhiy-storchaka
serhiy-storchaka reviewed May 11, 2019
View reviewed changes
Comment thread Modules/mathmodule.c Outdated
Comment thread Modules/mathmodule.c Outdated
Comment thread Modules/mathmodule.c Outdated
Comment thread Modules/mathmodule.c
Comment thread Modules/mathmodule.c
Comment thread Modules/mathmodule.c Outdated
mdickinson
mdickinson commented May 11, 2019
View reviewed changes
Comment thread Doc/library/math.rst Outdated
serhiy-storchaka
serhiy-storchaka reviewed May 11, 2019
View reviewed changes
Comment thread Modules/mathmodule.c
Comment thread Modules/mathmodule.c Outdated
mdickinson added 3 commits May 11, 2019 21:28
@mdickinson
Add missing error check; don't try to DECREF a NULL a
954588a
@mdickinson
Simplify some error returns.
a7c6cdb
@mdickinson
Another two test cases:
3c3a50e 
- clarify that floats are rejected even if they happen to be
  squares of small integers
- TypeError beats ValueError for a negative float
serhiy-storchaka
serhiy-storchaka approved these changes May 12, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. I did not analyze the original formulas, but the C code looks correct.

I left just few minor suggestions to the documentation. You can ignore them.

Comment thread Doc/library/math.rst Outdated
Comment thread Doc/library/math.rst Outdated
@serhiy-storchaka
Copy link
Copy Markdown
Member

serhiy-storchaka commented May 12, 2019

I added suggestions to add \ , but I just checked that this is not needed. Ignore these comments.

@mdickinson mdickinson merged commit 73934b9 into python:master May 18, 2019
@mdickinson mdickinson deleted the math-isqrt branch May 18, 2019 11:30
@mdickinson
Copy link
Copy Markdown
Member Author

mdickinson commented May 18, 2019

Merged. @serhiy-storchaka: many thanks for the thorough review!

@mdickinson mdickinson added the type-feature A feature request or enhancement label May 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

No one assigned

Labels

type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mdickinson @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot