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

System.TimeOnly: Compact round-trip format #68348

Open
CodeBlanch opened this issue Apr 21, 2022 · 15 comments
Open

System.TimeOnly: Compact round-trip format #68348

CodeBlanch opened this issue Apr 21, 2022 · 15 comments

Comments

@CodeBlanch
Copy link
Contributor

Today for culture-invariant round-tripping TimeOnly supports the o/O format.

Console.WriteLine(new TimeOnly(0, 1, 0).ToString("O")) // Outputs: "00:01:00.0000000"

It would be nice if there was a more compact format akin to TimeSpan c format basically hh:mm:ss[.fffffff] for TimeOnly.

Something like:

Console.WriteLine(new TimeOnly(0, 1, 0).ToString("c")) // Outputs: "00:01:00"

The benefit of this format would be to save some bits during transport when fractional portions of the time are not used.

Somewhat relates to #53539 & #53768

/cc @tarekgh

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 21, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@tarekgh
Copy link
Member

tarekgh commented Apr 21, 2022

can't you just use hh':'mm':'ss?

static string C = "hh':'mm':'ss";
TimeOnly.FromDateTime(DateTime.Now).ToString(C)

Note, TimeOnly is different than TimeSpan because it must get the formats from the used culture. We have t for culture aware short time. Therefore you'll never see C with DateTime/DateTimeOffset/TimeOnly/DateOnly. Supporting C will need to define if we use cultural separators or always use the : and '/`. I don't think this is worth adding as the workaround is really simple.

@ghost
Copy link

ghost commented Apr 21, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Today for culture-invariant round-tripping TimeOnly supports the o/O format.

Console.WriteLine(new TimeOnly(0, 1, 0).ToString("O")) // Outputs: "00:01:00.0000000"

It would be nice if there was a more compact format akin to TimeSpan c format basically hh:mm:ss[.fffffff] for TimeOnly.

Something like:

Console.WriteLine(new TimeOnly(0, 1, 0).ToString("c")) // Outputs: "00:01:00"

The benefit of this format would be to save some bits during transport when fractional portions of the time are not used.

Somewhat relates to #53539 & #53768

/cc @tarekgh

Author: CodeBlanch
Assignees: -
Labels:

area-System.Runtime, untriaged

Milestone: -

@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Apr 21, 2022
@tarekgh tarekgh added this to the Future milestone Apr 21, 2022
@CodeBlanch
Copy link
Contributor Author

You can do it with a custom format string, yes. I think the format would need to be HH':'mm':'ss.FFFFFFF though.

Basically this issue is because I think this would be useful/common enough that it should be a standard format. We can just leave this out there to see if anyone else feels the same. #53539 is proposing the same format which will probably need the support to go into #53768.

PS: This format would be for round-tripping so it would need to be culture invariant. Just a more compact version of O/o.

@tarekgh
Copy link
Member

tarekgh commented Apr 21, 2022

I think the format would need to be HH':'mm':'ss.FFFFFFF though.

You are right if you want to get closer to O :-) isn't HH':'mm':'ss.FFFFFFF will give you the same O output?

I am wondering why O is not enough for your scenario.

@tarekgh tarekgh added the needs-author-action An issue or pull request that requires more info or actions from the author. label Apr 21, 2022
@ghost
Copy link

ghost commented Apr 21, 2022

This issue has been marked needs-author-action since it may be missing important information. Please refer to our contribution guidelines for tips on how to report issues effectively.

@CodeBlanch
Copy link
Contributor Author

Here are a few examples. Basically the fractional units get compacted or are not written at all, depending on the value:

string format = "HH':'mm':'ss.FFFFFFF";
Console.WriteLine(TimeOnly.FromTimeSpan(TimeSpan.FromMinutes(1)).ToString(format)); // 00:01:00
Console.WriteLine(TimeOnly.FromTimeSpan(TimeSpan.FromMilliseconds(1)).ToString(format)); // 00:00:00.001
Console.WriteLine(TimeOnly.FromTimeSpan(TimeSpan.FromTicks(1)).ToString(format)); // 00:00:00.0000001

Console.WriteLine(TimeOnly.FromTimeSpan(TimeSpan.FromMinutes(1)).ToString("O")); // 00:01:00.0000000
Console.WriteLine(TimeOnly.FromTimeSpan(TimeSpan.FromMilliseconds(1)).ToString("O")); // 00:00:00.0010000
Console.WriteLine(TimeOnly.FromTimeSpan(TimeSpan.FromTicks(1)).ToString("O")); // 00:00:00.0000001

@ghost ghost added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Apr 21, 2022
@tarekgh tarekgh removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Apr 21, 2022
@tarekgh
Copy link
Member

tarekgh commented Apr 21, 2022

I am wondering about your scenario that needs the compact form and the full form is not acceptable. Note, most of the time, TimeOnly will not have such milliseconds without smaller fraction as you see in TimeSpan especially when dealing with system time which I guess will be your scenario. So, it is unlikely to run into cases need to compact the output.

@CodeBlanch
Copy link
Contributor Author

You kind of lost me there. You are saying the precision is NOT likely to be used with TimeOnly? I would agree there. But wouldn't that be an argument FOR the compacted format? If we agree the precision is not likely to be used we should also agree that it should be optional during round-trip 😄

@tarekgh
Copy link
Member

tarekgh commented Apr 22, 2022

What I mean is when working with system time, the time precision is higher than milliseconds. Compact formatting of such time wouldn't be noticeable. Here is an example which uses the compact pattern.

            string format = "HH':'mm':'ss.FFFFFFF";
            for (int i = 0; i < 1000000; i++)
            {
                TimeOnly to = TimeOnly.FromDateTime(DateTime.Now);
                Console.WriteLine($"{to.ToString(format)}");
            }

Out something like:

09:49:27.7696078
09:49:27.7697399
09:49:27.7698547
09:49:27.7699744
09:49:27.7700997
09:49:27.7702792
09:49:27.7704194
09:49:27.7705359
09:49:27.7707199
09:49:27.7708429
09:49:27.7709553
09:49:27.7710782
09:49:27.7712189
09:49:27.7713536
09:49:27.771458
09:49:27.7715862
09:49:27.7717099
09:49:27.7718269
09:49:27.7719315
09:49:27.7720225
09:49:27.7721371
09:49:27.7722448
09:49:27.7724362

Compact formatting is not helping much with that.

My point is, such compact formatting makes more sense for TimeSpan than TimeOnly.

@CodeBlanch
Copy link
Contributor Author

My point is, such compact formatting makes more sense for TimeSpan than TimeOnly.

👍

@julealgon
Copy link

What I mean is when working with system time, the time precision is higher than milliseconds.

Sure but what if you are not working with system time though? Or what if you are working with "time since the beginning of the day" instead of "time since now"?

I don't quite understand why creating a specific scenario where milliseconds will unlikely be 0 invalidates this request for other scenarios where it will be.

I can see a multitude of common use cases where the TimeOnly values will be rounded values and the problem described in the OP would make a lot of sense. For instance, imagine a scheduling API of sorts where you can define time slots for meetings. No one schedules meetings with millisecond precision.

@tarekgh
Copy link
Member

tarekgh commented Apr 25, 2022

@julealgon the other scenarios you are talking about here which mostly not having a fraction of milliseconds. I am wondering why using HH':'mm':'ss or HH':'mm':'ss.FFFFFFF formats is not enough? I am seeing for the TimeOnly, exposing compact format is really something nice to have thing more than crucial feature to add.

@julealgon
Copy link

I am seeing for the TimeOnly, exposing compact format is really something nice to have thing more than crucial feature to add.

I don't disagree with that, but I don't think something just being nice to have instead of crucial invalidates the request is my point.

@tarekgh
Copy link
Member

tarekgh commented Apr 25, 2022

I don't disagree with that, but I don't think something just being nice to have instead of crucial invalidates the request is my point.

It doesn't. We still have the issue open :-) but it is a low priority though as the workaround is quite simple.

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

No branches or pull requests

4 participants