-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Simplify input to AutoMLStep #529
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
purnesh42H
left a comment
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.
Don't include config.json file
delete details
keep the same
dataders
left a comment
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 made some commits to make it fit the existing repo. As I understand it, this isn't the real repo though so does getting this approved even matter? I'm just proposing changes to this notebook. @rastala
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.
A check is needed to see if output is specified(not None) as the steps don't enforce it.
|
@swanderz Please continue to propose changes. Even though the publishing mechanism is different, we could adopt your changes. |
|
@sanpil this PR is my feedback, can you please give feedback on the version of the notebook I'm proposing (not sure how to get see a helpful diff on Let me know what you think. If you understand and agree with my changes, I'm then willing to work with @purnesh42H and @rastala to get the PR approved, but we're definitely not at that point yet. |
|
@purnesh42H and I will review the changes and will get back to you. Thanks @swanderz! |
This is something I did a few weeks ago as I was debugging my own pipeline. For me, this is a a clearer and more robust way to do train test split w/
AutoMLStepandPipelineDataobjects. The changes I made:os.path.join()instead of+to concat stringstestTrainSplitStephas 1 `PipelineData object instead of 4@sanpil @yanrez @j-martens let me know what y'all think