distance_of_time_in_words displays Rational instead of integer #656

Closed
lighthouse-import opened this Issue May 16, 2011 · 9 comments

Projects

None yet

4 participants

@lighthouse-import

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/3806
Created by Corey - 2011-02-19 14:02:24 UTC

When using the mathn ruby standard library, either directly or indirectly, the display of distance_of_time_in_words for time distances of one year or greater shows a Rational number instead of an integer.

For example:

$ script/console 
Loading development environment (Rails 2.3.5)
>> from_time = Time.now
=> Thu Jan 28 09:11:53 -0800 2010
>> helper.distance_of_time_in_words(from_time, from_time + 1.year + 3.days)
=> "about 1 year"
>> require 'mathn'
=> ["Prime"]
>> helper.distance_of_time_in_words(from_time, from_time + 1.year + 3.days)
=> "about 368/365 years"
>> 

I think that if line 92 in rails/actionpack/lib/action_view/helpers/date_helper.rb uses the round method on the result of the division for the distance_in_years value the display will be as expected.

@lighthouse-import

Imported from Lighthouse.
Comment by Rohit Arondekar - 2010-10-07 10:37:37 UTC

Marking ticket as stale. If this is still an issue please leave a comment with suggested changes, creating a patch with tests, rebasing an existing patch or just confirming the issue on a latest release or master/branches.

@lighthouse-import

Imported from Lighthouse.
Comment by Corey - 2010-10-08 01:59:48 UTC

From the latest 2.3.x release:

    $ script/console 
    Loading development environment (Rails 2.3.9)
    >> from_time = Time.now
    => Thu Oct 07 18:20:30 -0700 2010
    >> helper.distance_of_time_in_words(from_time, from_time + 1.year + 3.days)
    => "about 1 year"
    >> require 'mathn'
    => ["Prime"]
    >> helper.distance_of_time_in_words(from_time, from_time + 1.year + 3.days)
    => "about 368/365 years"
    >>

From the latest Rails release:

    $ rails console
    Loading development environment (Rails 3.0.0)
    >> from_time = Time.now
    => Thu Oct 07 18:28:08 -0700 2010
    >> helper.distance_of_time_in_words(from_time, from_time + 1.year + 3.days)
    => "about 1 year"
    >> require 'mathn'
    => ["Prime"]
    >> helper.distance_of_time_in_words(from_time, from_time + 1.year + 3.days)
    => "about 368/365 years"
    >>

I wanted to add a test case that would illustrate the problem. However, as it turned out requiring 'mathn' in the date_helper_test.rb also had it loaded for all of the tests that run after it and some of those then also failed. I don't know how to sandbox the loading of 'mathn' in the test suite so that other tests aren't affected.

However, for Fixnums the methods / and div give the same result, and that changing from / to div ensures that mathn's Rationals are not returned. I've created a patch from 2-3-stable which does that for distance_of_time_in_words and attached it.

@lighthouse-import

Imported from Lighthouse.
Comment by Rohit Arondekar - 2010-10-08 02:07:01 UTC

Wait, so mathn is redefining '/'? I think then it's the gems fault and not Rails. I mean Rails shouldn't change because of this. Still, opening ticket to get a consensus on this issue. Thanks for the reply!

@lighthouse-import

Imported from Lighthouse.
Comment by Corey - 2010-10-08 02:34:36 UTC

Unfortunately, in my application I'm not loading 'mathn' directly, some gem I just happen to use is loading it. Everything was ok for me with until this change to the DateHelper.

Just for reference, here's the Ruby issue related to mathn's behavior.

@lighthouse-import

Imported from Lighthouse.
Comment by Ryan Bigg - 2010-10-09 20:57:33 UTC

Automatic cleanup of spam.

@lighthouse-import

Attachments saved to Gist: http://gist.github.com/971658

@pond
pond commented Jun 1, 2011

I have just been bitten by this too. I've no idea what code might be including "mathn". I just saw Rails break in an obscure fashion from one release to another.

I can't understand why the Rails team resist fixing this by changing to "div", whoever's fault they care to say it might be, when changing to "div" makes the code work properly with the wider Ruby universe at no cost to Rails itself. You were even sent a patch! Just apply it, and save countless developers the hassle of trying to tackle this bug independently.

@jasonhutchens

We've just been bitten by this. In our case, mathn leaks out of ruby-units (see olbrich/ruby-units#43 (comment)). Perhaps it would make sense for rails to emit a warning on startup if mathn is detected (along the lines that stuff might get broken when integer division is redefined to return a rational, for example).

@rafaelfranca
Member

This issue is already fixed on master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment