Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time represented as float? #9

Open
pnathan opened this issue Dec 16, 2017 · 1 comment
Open

Time represented as float? #9

pnathan opened this issue Dec 16, 2017 · 1 comment

Comments

@pnathan
Copy link

pnathan commented Dec 16, 2017

Per ISO8601.mli

    (** {2 Date parsing}

        [date], [time] and [datetime] parse a [string] and return the
        corresponding timestamp (as [float]).

        Times are {b always} converted to UTC representation.

        For each of these functions, a [_lex] suffixed function read
        from a [Lexing.lexbuf] instead of a [string].

        [datetime] functions also take an optional boolean [reqtime]
        indicating if parsing must fail if a date is given and not
        a complete datetime. (Default is true).

        Functions with [_tz] in their name return a [float * float option]
        representing [timestamp * offset option (timezone)]. [timestamp]
        will be the UTC time, and [offset option] is just an information
        about the original timezone. 
     *)

Why are timestamps represented as floats? Shouldn't there be an integer triple at the least for all of these(epoch in seconds, epoch fractions of second, and timezone)? Floats have losses of precision. A key use of ISO8601 is to allow locale-aware representation as opposed to epochs (which doesn't).

I would propose revising the interface to, rather than returning a float * float, returning one of these:

type instant = OffsetInstant of int * int * int | Instant of int * int

This would allow a precise buildup around the type. I am almost tempted to think, however, that this would be a better representation:

class cinstant (epoch: int) (fractional: int) (offset: int) = object(self) end

since frequently the use of methods on instants is handy (e.g., subtraction, parsing, extracting specific information) ; a direct modeling would be handy.

thoughts? I am OK submitting a PR here, I think.

@sagotch
Copy link
Collaborator

sagotch commented Dec 18, 2017

I'll try to look at it during the end of the week.

Probably that I did not need that much precision while writing this lib.

Also, keeping it very simple to use is important to me. I'll give you a more complete answer later, I have no time right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants