This project is archived and is in readonly mode.
using include? on a time range yields unhappy results
Reported by eydaimon | October 12th, 2010 @ 09:07 PM | in 3.x
Example:
Loading development environment (Rails 3.0.0)
ruby-1.9.2-p0 > (1.day.ago..Time.now).include? 3.hours.ago
succ is obsolete; use time + 1
/Users/daniel/.rvm/gems/ruby-1.9.2-p0/gems/activesupport-3.0.0/lib/active_support/time_with_zone.rb:316: warning: Time#
succ is obsolete; use time + 1
/Users/daniel/.rvm/gems/ruby-1.9.2-p0/gems/activesupport-3.0.0/lib/active_support/time_with_zone.rb:316: warning: Time#
succ is obsolete; use time + 1
/Users/daniel/.rvm/gems/ruby-1.9.2-p0/gems/activesupport-3.0.0/lib/active_support/time_with_zone.rb:316: warning: Time#
succ is obsolete; use time + 1
..... ad infinitum
solution is obviously to convert each item into an integer.
Comments and changes to this ticket
-
Aditya Sanghi October 12th, 2010 @ 10:47 PM
- Importance changed from to Low
Just to confirm, this is a problem only in 1.9.2, on 1.8.7 this code runs just fine.
-
eydaimon October 12th, 2010 @ 11:17 PM
- Tag changed from rails3.0.0, range, time to 1.9.2, rails3.0.0, range, time
-
Aditya Sanghi October 12th, 2010 @ 11:33 PM
- State changed from new to incomplete
- Milestone set to 3.x
- Assigned user set to Santiago Pastorino
- Tag changed from 1.9.2, rails3.0.0, range, time to rails3.0.0, range, time
I confirm that this indeed is a problem on 1.9.2.
@eydaimon, feel free to submit patches.
You may want to poke around in -
activesupport/lib/active_support/core_ext/range/include_range.rb
We would also need tests for this added of course.
Marking this incomplete, until we have patches.
-
Aditya Sanghi October 12th, 2010 @ 11:34 PM
- Assigned user cleared.
-
Aditya Sanghi October 18th, 2010 @ 03:42 PM
@eydaimon
I reexamined your request and read this thread again and looked a bit harder at the issue.
The "ad infinitum" is not really upto infinity its actually stepping thru EACH second between the upper bound and lower bound of your range. So even if this works, i would imagine it would be horrible in terms of performance. You get a sense of infinity because there is an error message with each step (a warning).
Now in Ruby 1.9.2 Range's include? calls succ on each element of the range. Outside of Rails, iteration over time is not possible but in Rails we have TimeWithZone with does seem to allow it.
One way to fix it would be to add the
succ
method in TimeWithZone class.activesupport/lib/time_with_zone.rb
def succ self.+(1) end
I added this in my local install of ruby and i got the expected result like you had in your original post.
I need to probably dig deeper as to if this is really necessary or how else to solve this problem and write tests too.
-
Szymon Nowak October 18th, 2010 @ 07:44 PM
I don't really think it's Rails issue. Just use Range#cover? instead:
(1.day.ago..Time.now).cover? 3.hours.ago => true
However, it's Ruby 1.9 only. If you're still on 1.8 you can simply alias cover? to include? and use cover? everywhere. Maybe Rails could do it for you...
Here's a bit more about it: http://rhnh.net/2009/08/03/range-include-in-ruby-1-9
-
Aditya Sanghi October 19th, 2010 @ 11:13 AM
- State changed from incomplete to invalid
- Tag changed from rails3.0.0, range, time to 1.9.2, rails3.0.0, range, time
Thanks Szymon! Didnt know about the 1.9 changes.
So the short of it is use cover? in 1.9 instead of include? and you should be good to go.
Apart from your own code, if you see anywhere in Rails code where this is happening, please let us know and we'll reopen this ticket. Marking invalid for now.
-
eydaimon October 19th, 2010 @ 04:34 PM
I said from the beginning the solution was just to use
.to_i
on the date fields. I don't see how it's invalid. It's unexpected behavior.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>