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::Span #to_json #from_json #4446

Closed
wants to merge 1 commit into from

Conversation

kostya
Copy link
Contributor

@kostya kostya commented May 22, 2017

No description provided.

@jhass
Copy link
Member

jhass commented May 22, 2017

In general +1, but why seconds over milli- or even microseconds?

@kostya
Copy link
Contributor Author

kostya commented May 22, 2017

not sure what is better, but seconds would use imo less string length.

@kostya
Copy link
Contributor Author

kostya commented May 22, 2017

this load is quite useful for me because, i have config with many float numbers, which i just convert to Time::Span on fly when load it from json.

@RX14
Copy link
Contributor

RX14 commented May 22, 2017

No it wouldn't, for the same precision. You have an extra character for the decimal point.

We shouldn't provide standard json conversions for data types which have no standard representation. Time is ok as it has iso8601, but there is no standard representation for time spans that is a safe default. We should add a series of converters for time spans but I don't think a default is sensible.

@kostya
Copy link
Contributor Author

kostya commented May 22, 2017

1.second => "1.0" or "1000.0" or "1000000.0" microseconds much bigger.
and in my config file, which i receive from another language, all intervals in seconds.

@RX14
Copy link
Contributor

RX14 commented May 22, 2017

@kostya except: for intervals measured using Time.now, the value is unlikely to be divisible by 10, meaning the decimal point makes the value longer and the raw number of ticks makes a lot more sense, especially for values under 100ms.

As I said, what representation you want to use isn't clear cut so the stdlib shouldn't make that decision. It should make it easy for the programmer to make that decision by providing a sensible selection of converters.

@mverzilli
Copy link

mverzilli commented Jun 2, 2017

Turns out ISO8601 defines how to represent durations. It might make sense to use that as the stdlib's to_json and from_json implementations.

See: https://en.wikipedia.org/wiki/ISO_8601#Durations

@mverzilli
Copy link

XML Schema uses that: https://www.w3.org/TR/xmlschema-2/#duration

And C# serializes TimeSpan's complying to that.

@RX14
Copy link
Contributor

RX14 commented Jun 2, 2017

From ISO 8601:

A time-interval shall be expressed in one of the following ways:
a) by a start and an end;
b) by a duration not associated with any start or end;
c) by a start and a duration;
d) by a duration and an end.

Looks like Wikipedia is wrong: ISO8601 is the way to go.

PDF of ISO8601 is here: http://xml.coverpages.org/ISO-FDIS-8601.pdf

@kostya
Copy link
Contributor Author

kostya commented Jun 3, 2017

this iso quite big to implement, i think i would use float in my code :)

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

Successfully merging this pull request may close these issues.

None yet

4 participants