97jaz / gregor

Date and time library for Racket
45 stars 10 forks source link

period-between fails with internal contract violation on certain inputs #14

Closed jackfirth closed 8 years ago

jackfirth commented 8 years ago

On Racket 6.4, the following program:

(period-between
  (moment 1970 1 1 #:tz 0)
  (moment 1969 12 31 16 #:tz "America/Los_Angeles")))

Fails with an internal contract violation:

string->immutable-string: contract violation
  expected: string?
  given: 0
jackfirth commented 8 years ago

I found a simpler example that seems to have the same root cause:

(+period (moment 1970 #:tz 0) empty-period)

This failure seems to happen whenever a moment is constructed using a number for a timezone instead of a string. These all fail as well:

(+period (moment 1970 #:tz 124) empty-period)
(+period (moment 1970 #:tz 0) (days 3))
(+period (moment 1970 #:tz 2498) (months 5))
97jaz commented 8 years ago

Looks to me like this comes from the offset resolver, which is using make-moment. However, it doesn't happen with:

(+months (moment 2000 #:tz 0) 5)

nor with

(+date-period (moment 2000 #:tz 0) (months 5))

This should tell me something. Er, let's see...

Yeah, the error actually occurs when the time-period is added (which is odd, since the offset resolver should be irrelevant to that process).

...

Oh, I see. The bug is in moment-add-nanoseconds. So this should also occur with any time arithmetic on moments with offset TZs, too. (So, no, it does not come from the offset resolver as wrote above.)

(+hours (moment 2000 #:tz 0) 5)

Yep. Well, that's a gaping hole in the test suite. Okay, the fix should be straightforward enough. As (I think) I mentioned in my email from yesterday, I'm considering making the datetime + offset and datetime + zone-id struct types separate in the new datetime library. I was never terribly happy with the current design.

97jaz commented 8 years ago

Correction: the bug is in posix->moment.

acarrico commented 7 years ago

Is this the same issue?

> (+date-period (minutes 4) (days 2))
#<period of 2 days, 4 minutes>
> (+date-period (days 2) (minutes 4))
; +date-period: contract violation
;   expected: Period-dp?
;   given: #<period of 4 minutes>
;   in: the p argument of
;       (->i
;        ((d date-arithmetic-provider?)
;         (p Period-dp?))
;        (#:resolve-offset
;         (resolve
;          (->
;           (or/c tzgap? tzoverlap?)
;           DateTime?
;           string?
;           (or/c Moment? #f)
;           Moment?)))
;        (r date-arithmetic-provider?))
;   contract from: 
;       <pkgs>/gregor-lib/gregor/main.rkt
;   blaming: top-level
;    (assuming the contract is correct)
;   at: <pkgs>/gregor-lib/gregor/main.rkt:160.2
$ raco pkg catalog-show gregor
Package name: gregor
 Author: zeppieri@gmail.com
 Source: git://github.com/97jaz/gregor/?path=gregor
 Checksum: 0d7c40c8d6ce6165419564df3b168821c322e3ae
 Tags: calendar, date, time
 Description: Date and time library
 Dependencies:
  gregor-lib
  gregor-doc
  base
97jaz commented 7 years ago

No, this isn't a bug. (minutes 4) isn't a date-period. It's a time-period. (It's also a period.)

So, there's really just one period struct type. Periods that only contain date fields (viz., years, months, weeks, and days) will satisfy the date-period? predicate. Periods that only contain time fields (viz., hours, minutes, seconds, milliseconds, microseconds, and nanoseconds) will satisfy time-period?. Periods that contain both date and time fields will satisfy period? but will not satisfy either of the other two predicates. When you use +date-period, the first argument has to satisfy date-arithmetic-provider? (that is, it has to be something that you can add date data to), and the second has to satisfy date-period?. In your failing example, the second argument is (minutes 4), which is not a date period.

97jaz commented 7 years ago

By the way, the reason for the distinction between date and time period is because date arithmetic isn't reducible to time arithmetic. For example, adding one day to a date is not the same thing as adding 24 hours. (It may have the same result in most cases, but not in all.)

97jaz commented 7 years ago

Well, there is a bug in the above, which I just noticed. The error message is using the internal name of the predicate (Period-dp?) instead of the public name (date-period?). That's unfortunate. In fact it's using a number of internal names. Ugh.

acarrico commented 7 years ago

Ah, now I have successfully navigated the docs and found my way to +time-period, which is what I'm looking for. This is a great library. I appreciate your work. Thanks, and sorry for hijacking this issue.