-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add DateTime to DateTime standard conversion. #4273
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4273 +/- ##
==========================================
+ Coverage 74.48% 74.57% +0.08%
==========================================
Files 877 878 +1
Lines 153663 154026 +363
Branches 16828 16852 +24
==========================================
+ Hits 114457 114863 +406
+ Misses 34474 34426 -48
- Partials 4732 4737 +5
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕐
@@ -1673,5 +1674,9 @@ public void Convert(in TX src, ref SB dst) | |||
public void Convert(in BL src, ref R8 dst) => dst = System.Convert.ToDouble(src); | |||
public void Convert(in BL src, ref BL dst) => dst = src; | |||
#endregion FromBL | |||
|
|||
#region ToDT | |||
public void Convert(in DT src, ref DT dst) => dst = src; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public void Convert(in DT src, ref DT dst) => dst = src; [](start = 8, length = 56)
Does not have test coverage as per https://codecov.io/gh/dotnet/machinelearning/pull/4273/diff #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a unit test for the new DateTime to DateTime conversion method.
In reply to: 330341048 [](ancestors = 330341048)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes an error which is caused by a missing DateTime to DateTime conversion when outputting DateTime columns in NimbusML. NimbusML calls in to
RowCursorUtils.GetGetterAsCore
(indirectly) which tries to find a conversion from DateTime to DateTime and fails with the following error:Error: *** System.InvalidOperationException: 'No standard conversion from 'DateTime' to 'DateTime'