FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-38639: Optimize floor(), ceil() and trunc() for floats. by serhiy-storchaka · Pull Request #16991 · python/cpython · GitHub

Repository navigation

bpo-38639: Optimize floor(), ceil() and trunc() for floats. - #16991

Merged
serhiy-storchaka merged 3 commits into
python:masterfrom
serhiy-storchaka:math-floor-float
Nov 16, 2019
Merged

serhiy-storchaka merged 3 commits into
python:masterfrom
serhiy-storchaka:math-floor-float

Conversation

serhiy-storchaka commented Oct 30, 2019 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Comment thread Modules/mathmodule.c Outdated

mdickinson left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Proposed changes LGTM. For things of exact type float (and possibly also for float subclasses), I think we could dispense with the math_1_to_int complications to get even more speedup. But that doesn't have to happen in this PR.

Do we have tests for the various cases of floats, float subclasses that don't override __floor__, float subclasses that do override __floor__, things that aren't floats but provide a __float__ method, etc.? If not, would it be worth adding such tests?

Copy link
Copy Markdown
Member

Do we have tests for the various cases ...

It occurs to me that I don't know how a subclass of float that implements its own __float__ should behave.

>>> class MyFloat(float):
...     def __float__(self):
...         return 9.0
... 
>>> x = MyFloat(16)
>>> math.sqrt(x)
4.0
>>> float(x)
9.0

Copy link
Copy Markdown
Member Author

I think that it would be better to not call __float__ for float subclasses, __int__ and __index__ for int subclasses, __complex__ for complex subclasses, __str__ for str subclasses, etc. We already have a content of corresponding class and can convert it to a C value or to a new Python instance.

Copy link
Copy Markdown
Member Author

Thank you for your review @mdickinson, I addressed your comments and added tests. Could you please take another look?

serhiy-storchaka merged commit 5fd5cb8 into python:master Nov 16, 2019
serhiy-storchaka deleted the math-floor-float branch November 16, 2019 16:00
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
mdickinson pushed a commit that referenced this pull request Feb 9, 2023
`math_1_to_whatever()` is no longer useful, since all existing uses of it convert to `float`.
Earlier versions of Python used `math_1_to_whatever` with an integer target; see
gh-16991 for the PR where that use was removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

4 participants


Back | FazBrowse Home | New Git URL