This project is archived and is in readonly mode.
DateTime#to_time should always return a Time
Reported by Magnus Holm | January 8th, 2009 @ 06:48 PM | in 2.x
A method called #to_time should always return a Time (or raise error) IMO.
Patch w/test: http://github.com/judofyr/rails/...
Comments and changes to this ticket
-
Geoff Buesing January 11th, 2009 @ 08:23 PM
It's worked this way for all 2.x releases; we'd really need a good reason to change this behavior now.
Do you have a specific use case where you run into an issue with this behavior?
-
Pratik March 12th, 2009 @ 02:41 PM
- Assigned user set to Geoff Buesing
-
Magnus Holm March 12th, 2009 @ 04:52 PM
Well, I have: I our app we need to turn all DateTime objects to Time objects and right now it's not possible. We've just created a helper, so it doesn't really matter to us, but it's kinda weird DateTime#to_time doesn't always return a Time (even though it's documentated).
Sometime in the future I think this needs to be merged; there's no point of not doing it when it's perfectly possible.
I've also included the patch as an attachment, in case I'll delete my Rails-repo :-)
-
Geoff Buesing March 14th, 2009 @ 04:44 PM
Your implementation does a time zone conversion, which is adding new behavior to this method. The resulting object is equivalent in a #to_i sense (i.e., seconds since epoch), but it's not equivalent in a time values sense -- here's an example to help explain:
# DateTime#to_time behavior with above patch: >> DateTime.now.to_s(:long) => "March 14, 2009 11:04" >> DateTime.now.to_time.to_s(:long) => "March 14, 2009 16:04"
The current behavior of DateTime#to_time only returns a Time object if it cleanly maps to an equivalent Time object in both a #to_i sense and a time values sense. Given that ActiveSupport ducktypes DateTime objects to respond like Time objects for most of the common methods we need use, you can still do most everything you want with the returned DateTime object, so this seems like a reasonable fallback.
Interestingly, Ruby 1.9's DateTime#to_time always converts to the system local zone, similar to your patch, just with a different zone:
irb(main):006:0> DateTime.now.to_time => 2009-03-14 11:20:01 -0500 irb(main):007:0> DateTime.now.new_offset(0).to_time => 2009-03-14 11:20:08 -0500
A reasonable argument could be made that we should be following this behavior instead of what's proposed here.
I do think the current ActiveSupport behavior works well for us, given the additional ducktyping support for DateTimes. Given this, and the fact that this patch would potentially cause unexpected behavior, I'm -1 on this.
-
Geoff Buesing March 29th, 2009 @ 09:44 PM
- State changed from new to wontfix
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>
People watching this ticket
Attachments
Referenced by
- 4919 distance_of_time_in_words should work with DateTime as argument due to this bug: https://rails.lighthouseapp.com/project...
- 4919 distance_of_time_in_words should work with DateTime as argument thus, if #1713 is a wontfix you get inconsistent behavior...