Skip to content
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

Fix integer/float remainder with infinity argument of opposite sign #4257

Merged
merged 1 commit into from Mar 12, 2021

Conversation

jeremyevans
Copy link
Contributor

Previously, the result was incorrect:

4.2.divmod(-Float::INFINITY)
Before: => [-1, -Infinity]
After: => [0, 4.2]

4.2.modulo(-Float::INFINITY) # or %
Before: => -Infinity
After: => 4.2

4.2.remainder(-Float::INFINITY)
Before: => NaN
After: => Infinity

Fixes [Bug #6120]

@marcandre
Copy link
Member

marcandre commented Mar 10, 2021

I'm always confused with modulo and negative numbers 😅

Given that:

4.2.divmod(-10000)
=> [-1, -9995.8]`
4.2.modulo(-10000)
=> -9995.8
4.2.remainder(-10000)
=> 4.2

I think that divmod and modulo are ok as is, and

4.2.remainder(-Float::INFINITY)
Before: => NaN
After: => 4.2

Previously, the result was incorrect:

4.remainder(-Float::INFINITY)
Before: => NaN
After: => 4

4.2.remainder(-Float::INFINITY)
Before: => NaN
After: => 4.2

Fixes [Bug ruby#6120]
@jeremyevans jeremyevans force-pushed the numeric-divmod--infinity-6120 branch from 858c568 to 1eac3a8 Compare March 10, 2021 23:10
@jeremyevans
Copy link
Contributor Author

Obviously I get confused with them as well. New commit pushed that only affects remainder and returns the receiver if the argument is infinite.

@jeremyevans jeremyevans changed the title Fix integer/float divmod/modulo/%/remainder with infinity argument of opposite sign Fix integer/float remainder with infinity argument of opposite sign Mar 10, 2021
Copy link
Member

@marcandre marcandre left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jeremyevans jeremyevans merged commit aaab3b1 into ruby:master Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants