Closed mco-gh closed 3 years ago
I realize you might or might not want to merge this PR, for example, you might want to retain TRANSFORM for its educational value, even if it simplifies things slightly and makes the model exportable. Anyway, the primary purpose of this PR is for you to double check my code changes. If you do want to merge it, I'll revert the project id back to a placeholder.
The code looks great to me. It simplifies the code and I can see how TRANSFORM is not necessarily a crucial step in the overall workflow, because even without exporting the model for online prediction, users would still need to do the required joins and re-engineer the appropriate features to make predictions.
Thanks for adding in relevant comments in your code. I've re-added the project id placeholder and I'm happy to merge.
Glad it's good for you, thanks for the review! I don't think I have the power to merge it, do you?
On Tue, 4 May 2021 at 11:52, Polong Lin @.***> wrote:
The code looks great to me. It simplifies the code and I can see how TRANSFORM is not necessarily a crucial step in the overall workflow, because even without exporting the model for online prediction, users would still need to do the required joins and re-engineer the appropriate features to make predictions.
Thanks for adding in relevant comments in your code. I've re-added the project id placeholder and I'm happy to merge.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/GoogleCloudPlatform/analytics-componentized-patterns/pull/58#issuecomment-831851714, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFAXF77X7XJ6CZAH3LZDFLTL7GWBANCNFSM435FOOWA .
-- Marc Cohen @.*** http://about.me/marc1
this also speeds up pre-processing because it avoids a lot of timestamp conversion during churn/bounce calculations