-
Notifications
You must be signed in to change notification settings - Fork 16
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 Coerce #37
Add Coerce #37
Conversation
Codecov Report
@@ Coverage Diff @@
## master #37 +/- ##
==========================================
+ Coverage 92.67% 92.69% +0.02%
==========================================
Files 17 18 +1
Lines 437 452 +15
==========================================
+ Hits 405 419 +14
- Misses 32 33 +1
Continue to review full report at Codecov.
|
Co-authored-by: Júlio Hoffimann <julio.hoffimann@gmail.com>
Co-authored-by: Júlio Hoffimann <julio.hoffimann@gmail.com>
Co-authored-by: Júlio Hoffimann <julio.hoffimann@gmail.com>
Co-authored-by: Júlio Hoffimann <julio.hoffimann@gmail.com>
Co-authored-by: Júlio Hoffimann <julio.hoffimann@gmail.com>
…rms.jl Pulling changes for Coerce transform
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.
Please add tests with the Categorical scitype as well. It is more tricky to get right.
Co-authored-by: Júlio Hoffimann <julio.hoffimann@gmail.com>
Co-authored-by: Júlio Hoffimann <julio.hoffimann@gmail.com>
@ceferisbarov we are almost there. We just need one more adjustment to this PR and also add the transform to the table of available transforms in the README. |
@ceferisbarov this branch has conflicts with the latest master branch. You need to resolve the conflicts in the PR before we can merge it. |
Thank you @ceferisbarov for the updates, it all looks good to me! @eliascarv do you have any additional suggestion before we merge this? |
Just saw you approved the PR @eliascarv , thank you! |
No description provided.