-
Notifications
You must be signed in to change notification settings - Fork 393
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
Use proper test ranges in feature converter test #143
Conversation
@ajayborra please have a look. |
Codecov Report
@@ Coverage Diff @@
## master #143 +/- ##
=========================================
+ Coverage 86.25% 86.4% +0.15%
=========================================
Files 299 299
Lines 9749 9748 -1
Branches 538 537 -1
=========================================
+ Hits 8409 8423 +14
+ Misses 1340 1325 -15
Continue to review full report at Codecov.
|
Lgtm @tovbinm , 👍the tuple feeding approch with |
(arrType(DoubleType), weakTypeTag[types.Geolocation], arrType(DoubleType)), | ||
(arrType(StringType), weakTypeTag[types.TextList], arrType(StringType)), | ||
(mapType(StringType), weakTypeTag[types.TextMap], mapType(StringType)), | ||
(mapType(DoubleType), weakTypeTag[types.RealMap], mapType(DoubleType)), |
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.
do you need map float, map int, map short, etc as well?
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.
I thought about it, but I dont think it's that important. Maybe in another PR.
Related issues
#115
Describe the proposed solution
This is a continuation of #135 PR by @ajayborra