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

Add the support of DateOnly and TimeOnly types in TryParse method Utf8Parser class #53768

Closed
goldytech opened this issue Jun 5, 2021 · 8 comments

Comments

@goldytech
Copy link

Background and Motivation

In .the current version of .NET (5.0.203) Utf8Parser class has only support for DateTime.
https://docs.microsoft.com/en-us/dotnet/api/system.buffers.text.utf8parser.tryparse?view=net-5.0#System_Buffers_Text_Utf8Parser_TryParse_System_ReadOnlySpan_System_Byte__System_DateTime__System_Int32__System_Char_

With .NET 6 Preview 4 two new types have been introduced DateOnly and TimeOnly. It would be good to have the support of these two types in the TryParse method of Utf8Parser class. In the current version if the date is only passed without time the out variable returns a null value. I believe there will be legitimate UseCases where only date and time parsing are required explicitly. In order to extract the DateTime now , developers have to fallback to Encoding.UTF8.GetString() and then use DateTime.TryParse() method to get the date / time value. This is resource intensive operation and allocations get increased.
Eg.

if (DateTime.TryParse(Encoding.UTF8.GetString(buffer, out var dateofJoining))

                           {
                                 record.DateOfJoining = dateofJoining;
                            }  

Proposed API

Additional two overload methods must be added to current Utf8Parser static class to support DateOnly and TimeOnly types.

Usage Examples

Eg
For DateOnly type

if (Utf8Parser.TryParse(buffer, out DateOnly value, out var bytesConsumed))
                                {
                                    record.DateOfJoining = value;
                                }

For TimeOnly type

if (Utf8Parser.TryParse(buffer, out TimeOnly value, out var bytesConsumed))
                                {
                                    record.ApplicationTime = value;
                                }

Alternative Designs

Risks

@goldytech goldytech added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 5, 2021
@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.

@ghost
Copy link

ghost commented Jun 5, 2021

Tagging subscribers to this area: @tarekgh, @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

In .the current version of .NET (5.0.203) Utf8Parser class has only support for DateTime.
https://docs.microsoft.com/en-us/dotnet/api/system.buffers.text.utf8parser.tryparse?view=net-5.0#System_Buffers_Text_Utf8Parser_TryParse_System_ReadOnlySpan_System_Byte__System_DateTime__System_Int32__System_Char_

With .NET 6 Preview 4 two new types have been introduced DateOnly and TimeOnly. It would be good to have the support of these two types in the TryParse method of Utf8Parser class. In the current version if the date is only passed without time the out variable returns a null value. I believe there will be legitimate UseCases where only date and time parsing are required explicitly. In order to extract the DateTime now , developers have to fallback to Encoding.UTF8.GetString() and then use DateTime.TryParse() method to get the date / time value. This is resource intensive operation and allocations get increased.
Eg.

if (DateTime.TryParse(Encoding.UTF8.GetString(buffer, out var dateofJoining))

                           {
                                 record.DateOfJoining = dateofJoining;
                            }  

Proposed API

Additional two overload methods must be added to current Utf8Parser static class to support DateOnly and TimeOnly types.

Usage Examples

Eg
For DateOnly type

if (Utf8Parser.TryParse(buffer, out DateOnly value, out var bytesConsumed))
                                {
                                    record.DateOfJoining = value;
                                }

For TimeOnly type

if (Utf8Parser.TryParse(buffer, out TimeOnly value, out var bytesConsumed))
                                {
                                    record.ApplicationTime = value;
                                }

Alternative Designs

Risks

Author: goldytech
Assignees: -
Labels:

api-suggestion, area-System.Globalization, untriaged

Milestone: -

@tarekgh tarekgh added area-System.Runtime and removed area-System.Globalization untriaged New issue has not been triaged by the area owner labels Jun 5, 2021
@ghost
Copy link

ghost commented Jun 5, 2021

Tagging subscribers to this area: @tannergooding
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

In .the current version of .NET (5.0.203) Utf8Parser class has only support for DateTime.
https://docs.microsoft.com/en-us/dotnet/api/system.buffers.text.utf8parser.tryparse?view=net-5.0#System_Buffers_Text_Utf8Parser_TryParse_System_ReadOnlySpan_System_Byte__System_DateTime__System_Int32__System_Char_

With .NET 6 Preview 4 two new types have been introduced DateOnly and TimeOnly. It would be good to have the support of these two types in the TryParse method of Utf8Parser class. In the current version if the date is only passed without time the out variable returns a null value. I believe there will be legitimate UseCases where only date and time parsing are required explicitly. In order to extract the DateTime now , developers have to fallback to Encoding.UTF8.GetString() and then use DateTime.TryParse() method to get the date / time value. This is resource intensive operation and allocations get increased.
Eg.

if (DateTime.TryParse(Encoding.UTF8.GetString(buffer, out var dateofJoining))

                           {
                                 record.DateOfJoining = dateofJoining;
                            }  

Proposed API

Additional two overload methods must be added to current Utf8Parser static class to support DateOnly and TimeOnly types.

Usage Examples

Eg
For DateOnly type

if (Utf8Parser.TryParse(buffer, out DateOnly value, out var bytesConsumed))
                                {
                                    record.DateOfJoining = value;
                                }

For TimeOnly type

if (Utf8Parser.TryParse(buffer, out TimeOnly value, out var bytesConsumed))
                                {
                                    record.ApplicationTime = value;
                                }

Alternative Designs

Risks

Author: goldytech
Assignees: -
Labels:

api-suggestion, area-System.Runtime

Milestone: -

@tarekgh tarekgh added this to the Future milestone Jun 5, 2021
@ghost
Copy link

ghost commented Jun 5, 2021

Tagging subscribers to this area: @tannergooding, @pgovind, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

In .the current version of .NET (5.0.203) Utf8Parser class has only support for DateTime.
https://docs.microsoft.com/en-us/dotnet/api/system.buffers.text.utf8parser.tryparse?view=net-5.0#System_Buffers_Text_Utf8Parser_TryParse_System_ReadOnlySpan_System_Byte__System_DateTime__System_Int32__System_Char_

With .NET 6 Preview 4 two new types have been introduced DateOnly and TimeOnly. It would be good to have the support of these two types in the TryParse method of Utf8Parser class. In the current version if the date is only passed without time the out variable returns a null value. I believe there will be legitimate UseCases where only date and time parsing are required explicitly. In order to extract the DateTime now , developers have to fallback to Encoding.UTF8.GetString() and then use DateTime.TryParse() method to get the date / time value. This is resource intensive operation and allocations get increased.
Eg.

if (DateTime.TryParse(Encoding.UTF8.GetString(buffer, out var dateofJoining))

                           {
                                 record.DateOfJoining = dateofJoining;
                            }  

Proposed API

Additional two overload methods must be added to current Utf8Parser static class to support DateOnly and TimeOnly types.

Usage Examples

Eg
For DateOnly type

if (Utf8Parser.TryParse(buffer, out DateOnly value, out var bytesConsumed))
                                {
                                    record.DateOfJoining = value;
                                }

For TimeOnly type

if (Utf8Parser.TryParse(buffer, out TimeOnly value, out var bytesConsumed))
                                {
                                    record.ApplicationTime = value;
                                }

Alternative Designs

Risks

Author: goldytech
Assignees: -
Labels:

api-suggestion, area-System.Buffers

Milestone: Future

@tarekgh
Copy link
Member

tarekgh commented Jun 5, 2021

#53539
#51302

@teo-tsirpanis
Copy link
Contributor

teo-tsirpanis commented Mar 15, 2022

And Half. And corresponding members in Utf8Formatter.

@tannergooding
Copy link
Member

@teo-tsirpanis could you open a separate proposal for Half?

@tannergooding tannergooding added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Sep 8, 2022
@bartonjs
Copy link
Member

bartonjs commented Jul 20, 2023

Video

Rather than adding things on Utf8Parser, the plan forward is to add the UTF-8 parse methods to the relevant types themselves (generally along with implementing IUtf8SpanParsable.

For DateOnly and TimeOnly, these API additions are already covered under #81500.

It came up in discussion that DateOnly (and maybe TimeOnly) might want more options than the simple parse, like DateTime.TryParseExact has, but those would just be overloads to the TryParse methods on their types, and still wouldn't affect Utf8Parser. As those also weren't part of this original proposal, they're left for someone else to do the research on and create a dedicated issue for.

@bartonjs bartonjs closed this as not planned Won't fix, can't repro, duplicate, stale Jul 20, 2023
@bartonjs bartonjs removed the api-ready-for-review API is ready for review, it is NOT ready for implementation label Jul 20, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants