Time and duration module

[srebhan]

This PR adds time and duration support to starlark and solves issue #19. The API is inspired by the discussion in above issue as well as the linked first implementation for skylark. However, we diverged on some points from the proposal (as well as from the python datetime API) to provide a more clean and consistent interface.

[alandonovan]

Hi Sven, many thanks for contributing this PR! It looks like a good start to a really useful package.

I have made a quick pass over the code, but it's tricky to review the big stack of PRs when all I want at this stage is to see the big picture. I suggest you flatten the stack, and put all the code in a single file. It's really not much code, it's very clean, and it has no existing constraints, so the precision of fine-grained commits isn't needed.

Initial comments:

• Let's move this to lib/time. I realize this is a break from the precedent of starlarkjson and starlarkstruct, but I plan to move them, and I have an open PR for lib/proto already.
• Why does Duration have a frozen boolean? It's immutable.
• I think the Duration accessors (hours, minutes) etc should be integers. Users can get the nanoseconds and do float division if they want fractional parts.
• There's no need for method doc comments about which interface is satisfied.
• Duration should implement Value directly. There's no need for a pointer (*Duration).
• If we support direction int we should also support int duration.
• let's support 'duration // duration' too. (Result: int)
• typo: "operand" not "operant"
• I don't think the operation 'duration + int' makes sense. Ditto minus.
• The result of duration / int should be another duration, not a float. Similarly duration / float.

Paging Brendan @b5 as author of https://github.com/qri-io/starlib/tree/master/time.

[srebhan]

@alandonovan thx for the mind-blowing fast review! Just for me to understand, with "reducing the stack" you mean squashing the commits or are we talking of merging the files (time.go, module.go, etc)?

[alandonovan]

Just for me to understand, with "reducing the stack" you mean squashing the commits or are we talking of merging the files (time.go, module.go, etc)?

Both. A single commit with a single file should be quite manageable. If you really want to split it into time.go and duration.go, do that, but our text editors and browsers are quite capable of handling book-length documents efficiently.

[b5]

👋 @alandonovan, sorry it's been a while. @srebhan I haven't had a chance to look at your code, but as Alan mentioned, we maintain a time package for go-starlark (among others) at github.com/qri-io/starlib. We've talked in the past about moving this package over to this repo, but haven't gotten to it.

I think settling on a uniform API for a time package in starlark is important for keeping confusion to a minimum, so it's time for us to get off our butts. @srebhan have you seen our work? If so, is there anything in it that's lacking for your use?

[alandonovan]

A single commit with a single file should be quite manageable.

Taking another look, the stack of commits is fine. I'm not sure what I clicked before, but I got lost in a stack of tiny edits. You may disregard my comment to flatten the stack.

[srebhan]

@b5 I didn't see your work before, sorry! And I'm not religious about which code goes in. :-) What I'm missing in your code (it can be added though) is a direct construction of the time/duration objects without parsing. For example, I have year, month and day from a data-source and what to construct an time object. With your code I would need to construct a string with the data I have to parse it afterwards right? Another missing function is getting the weekday of a date, but that can be added to your code easily.

So @b5, @alandonovan how to proceed? My code is not yet in production use, therefore I'm not bound to the API. However, there is a certain pressure to go forward with this on my side... :-)

[alandonovan]

Direct construction without parsing seems like a key feature, as does getting the weekday of a date. Let's add both.

I don't care who wrote the code (though either or both of Honda and qri.io would need to assign copyright to the Bazel Authors); I care only about the Go and Starlark interfaces that we expose. I suggest you (Sven) compare your code and Brendan's, understand any differences, and contribute a synthesis of the two.

[srebhan]

@b5 what do you think? Your code looks more mature and it already had more testing I guess. How about you prepare a PR and I add the two missing features on top of it? We only need to talk about the naming of the constructors vs. parsing functions...

[b5]

Sounds great! I'm with both on not caring about who's code gets used 😄, and would love to get the benefits of your work in there! I'll open a PR

[b5]

ok, just opened #327! cc @srebhan

[srebhan]

Closing as #327 was merged.