Project

General

Profile

Bug #13420

Integer#{round,floor,ceil,truncate} should always return an integer, not a float

Added by stomar (Marcus Stollsteimer) 2 months ago. Updated about 2 months ago.

Status:
Closed
Priority:
Normal
Target version:
-
[ruby-core:80645]

Description

The current behavior of Integer#{round,floor,ceil,truncate} produces wrong results for large integers.

In the case of a positive precision argument, these methods return the receiver converted to a float, effectively doing the same as to_f.

2.round(1)  # => 2.0

This leads to errors for large integers:

(10**25).round(2).to_i  # => 10000000000000000905969664
(10**400).round(2)      # => Infinity

Mathematically speaking, the value of an integer should not be changed by #round, #floor, #ceil, or #truncate, regardless of the precision (when positive). An integer rounded to e.g. 2 decimal digits is still the same (and exact) integer.

The desired behavior should be to keep precision as high as possible by not needlessly converting to Float.

The provided patch fixes these methods to return self for positive precision argument, similar to the behavior without a specified precision, i.e. precision 0.

0001-Integer-round-floor-ceil-truncate.patch View (7.83 KB) stomar (Marcus Stollsteimer), 04/11/2017 06:31 AM


Related issues

Related to Ruby trunk - Bug #12780: BigDecimal#round returns different types depending on argument Assigned

Associated revisions

Revision 58586
Added by stomar (Marcus Stollsteimer) about 2 months ago

make Integer#{round,floor,ceil,truncate} always return integer

  • numeric.c (int_round): return integer (self) instead of float for Integer#round with positive ndigits argument, because conversion to float introduces errors for large integers.
  • numeric.c (int_floor): ditto for Integer#floor.
  • numeric.c (int_ceil): ditto for Integer#ceil.
  • numeric.c (int_truncate): ditto for Integer#truncate.

  • test/ruby/test_integer.rb: adjust test cases and add some more.

[Bug #13420]

Revision 58589
Added by stomar (Marcus Stollsteimer) about 2 months ago

NEWS: Integer#{round,floor,ceil,truncate} [Bug #13420]

History

#1 [ruby-core:80649] Updated by nobu (Nobuyoshi Nakada) 2 months ago

Indeed.
And documents?

#2 Updated by stomar (Marcus Stollsteimer) 2 months ago

  • File deleted (0001-Integer-round-floor-ceil-truncate.patch)

#3 [ruby-core:80653] Updated by stomar (Marcus Stollsteimer) 2 months ago

Patch updated to include doc fixes.

#4 Updated by mrkn (Kenta Murata) 2 months ago

  • Related to Bug #12780: BigDecimal#round returns different types depending on argument added

#5 [ruby-core:80815] Updated by stomar (Marcus Stollsteimer) 2 months ago

The patch has been updated to also include doc changes.

Any thoughts on this?

#7 [ruby-core:81007] Updated by stomar (Marcus Stollsteimer) about 2 months ago

nobu, does this mean "Nice. Go ahead and apply!"?

(I fear I'm not too familiar with the ways around here, yet.)

And I noticed that while Integer#{floor,ceil,truncate} accept an ndigits argument only since recently (Ruby 2.4), Integer#round has this option and the behavior of returning a float for quite some time, and it's also in the specs (spec/rubyspec/core/integer/round_spec.rb). I guess they would need to get updated.

#8 [ruby-core:81010] Updated by nobu (Nobuyoshi Nakada) about 2 months ago

  • Status changed from Open to Assigned
  • Assignee set to stomar (Marcus Stollsteimer)

stomar (Marcus Stollsteimer) wrote:

nobu, does this mean "Nice. Go ahead and apply!"?

Yes, of course.

And I noticed that while Integer#{floor,ceil,truncate} accept an ndigits argument only since recently (Ruby 2.4), Integer#round has this option and the behavior of returning a float for quite some time, and it's also in the specs (spec/rubyspec/core/integer/round_spec.rb). I guess they would need to get updated.

Indeed, it needs a version guard.

#9 Updated by stomar (Marcus Stollsteimer) about 2 months ago

  • Status changed from Assigned to Closed

Applied in changeset trunk|r58586.


make Integer#{round,floor,ceil,truncate} always return integer

  • numeric.c (int_round): return integer (self) instead of float for Integer#round with positive ndigits argument, because conversion to float introduces errors for large integers.
  • numeric.c (int_floor): ditto for Integer#floor.
  • numeric.c (int_ceil): ditto for Integer#ceil.
  • numeric.c (int_truncate): ditto for Integer#truncate.

  • test/ruby/test_integer.rb: adjust test cases and add some more.

[Bug #13420]

#10 [ruby-core:81011] Updated by stomar (Marcus Stollsteimer) about 2 months ago

Applied.

I opened a PR for the specs: https://github.com/ruby/spec/pull/436 [merged]

Should I add a comment in NEWS or not? [done]

#11 [ruby-core:81012] Updated by Eregon (Benoit Daloze) about 2 months ago

stomar (Marcus Stollsteimer) wrote:

Applied.
I opened a PR for the specs: https://github.com/ruby/spec/pull/436

Thanks!

Should I add a comment in NEWS or not?

Yes, I think the rule is to add a NEWS entry for every user-visible behavior change.

Also available in: Atom PDF