-
Notifications
You must be signed in to change notification settings - Fork 604
WIP: Add predeployed models to TabPy #245
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
Hello @sbabayan! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2019-05-10 23:03:59 UTC |
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.
Looks great!
…d in CONTRIBUTING.md that running the integration test will have side effects of installing packages on to the machine
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.
Comments in case they help
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.
+1 on olek's feedback, but once that's addressed ship it!
…so users can run setup for models without needing to run set_env, fixed model deployment due to logger errors
Oh, one thing - could you update |
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.
Nice job - this has really come a long ways. Just some more ideas in case they are helpful
Updated documentation, added an integration test, and implemented PCA/SentimentAnalysis