Ensure current_time returns a time (and not a Rational)

[emlyn]

Fixes #3145

Note: I've added a monkey-patched Time.inspect:

Printing a Time in Sonic Pi appears to print the result of calling .inspect on the time object. In most cases, this will print the sub-second part as a rational (e.g. 2022-07-21 23:14:06 3267887/4194304 +0100), which is not very intuitive. Therefore I monkey-patched the Time.inspect method to print the time as a decimal fraction to the nearest millisecond (so the previous example appears as 2022-07-21 23:14:06.779 +0100).

Leaving my initial notes below for context:

~~BEFORE MERGING, Note that there is some strange behaviour that I don't understand~~

For some reason, the fractional seconds are still printed as a rational:

puts Time.at(current_time) # 2022-07-21 23:12:38 8989/524288 +0100
#                                                ^^^^^^^^^^^

Even if I ensure it's converted to a float first:

puts Time.at(current_time.to_f) # 2022-07-21 23:13:25 3979463/4194304 +0100
#                                                     ^^^^^^^^^^^^^^^

Even explicitly converting a literal float to a Time:

puts Time.at(1658441646.779125) # 2022-07-21 23:14:06 3267887/4194304 +0100
#                                                     ^^^^^^^^^^^^^^^

Although I have found a few values are printed as a decimal (where the fractional part is an inverse power of two like 0.5, 0.25, 0.125... or small multiples of them like 0.75 or 0.875):

puts Time.at(1658441646.125) # 2022-07-21 23:14:06.125 +0100
#                                                 ^^^^

I tried with plain ruby on the command line, and there the fractional part is ignored:

> echo 'puts Time.at(1658441646.779125)' | /Applications/Sonic\ Pi.app/Contents/Resources/app/server/native/ruby/bin/ruby
2022-07-21 23:14:06 +0100

Anyone have any idea what's going on here?

---

I've investigated a bit, and it looks like it's normal that the sub-second part of a Time can be rational (docs). I think the values that were printed as decimal must be ones that are exactly representable in floating point.

It's still a bit strange that it's displayed as a ratio. Is there something in Sonic Pi that's changing the the way Times are formatted by default?

---

OK, I think I've worked out what's happening!

Printing the time is calling .inspect on it, which does print the sub-seconds as a rational (even from bare Ruby on the command line).

So this seems to be working as designed.

However, I do wonder whether it might be clearer to print out times with decimal fractions of seconds. One way to achieve this would be to monkey-patch the inspect method on Time like this (to print to nearest millisecond):

class Time
  def inspect()
    strftime "%Y-%m-%d %H:%M:%S.%L %z"
  end
end

Is that reasonable, or is there a better way? If it's OK, where would be the bets place to put this?

[jhaagmans]

You've entered an interesting realm here!

So, ruby is actually doing a very sensible thing here, although I agree with your comment about readability. Consider the classic 0.1 + 0.2 problem and you might understand why ruby is actually doing this. When you're at the point where you're concerned with nanoseconds, you wouldn't want time to stand still for a nanosecond, would you?

irb(main):001:0> Time.at(0.299999999) == Time.at(0.3)
=> false
irb(main):002:0> Time.at(0.299999999).nsec == Time.at(0.3).nsec
=> true

Anyway! I think you're on the right track with Time#at (since until this point, I expect the system needs this level of precision), but you might want to simply consider calling strftime in the current_time method itself instead of monkeypatching Time (which might harm debugging at some point in the application).

If would be nice if you could also update the docs to reflect this behaviour.

[emlyn]

I'm not sure that calling strftime in the current_time method is the best solution, because then it will return a string, and people will likely want to do other things with it (involving arithmetic).

I think a better way would be to return a simple wrapper class that inherits from Time but with inspect overridden, so it will print consistently in the logs but otherwise behave like a Time. I've updated the PR accordingly.

[jhaagmans]

Oh that is a good point, I wasn't considering a case where you'd use current_time for calculations in sonic pi itself. I believe the "bug" you found is more related to changes in ruby itself.

A class extension would definitely be much cleaner here.

[samaaron]

This is useful - thanks. Also great advice by @jhaagmans for using a class extension here - thanks!