-
Notifications
You must be signed in to change notification settings - Fork 35
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
Issue 112 Get Pipeline Inputs #117
Conversation
Codecov Report
@@ Coverage Diff @@
## master #117 +/- ##
=========================================
+ Coverage 78.58% 78.78% +0.2%
=========================================
Files 5 5
Lines 691 707 +16
=========================================
+ Hits 543 557 +14
- Misses 148 150 +2
Continue to review full report at Codecov.
|
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.
Great contribution @erica-chiu !
I added some comments to reduce a little bit the code duplication, but once this is addressed the code seems ready to go in.
Also remember about adding yourself to the AUTHORS list! :-)
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.
It looks good @erica-chiu !
Just one last thing before merging.
Would you mind changing the "Vertical" multi-line method calls to "Vertical Hanging Indent" or "Hanging Grid"?
You will see what I mean here: https://github.com/timothycrosley/isort#multi-line-output-modes
More specifically, using styles 3 or 4 instead of 1.
I know it is not an important change, but it is to keep a constant styling with the rest of the project.
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.
Thanks @erica-chiu !
Issue 103: Primitive for `sklearn.linear_model.MultiTaskLasso`
Resolves #112