-
Notifications
You must be signed in to change notification settings - Fork 42
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
docs: add predict sample to samples/snippets/bqml_getting_started_test.py #388
Conversation
Here is the summary of changes. You are about to add 2 region tags.
You are about to delete 1 region tag.
This comment is generated by snippet-bot.
|
} | ||
) | ||
# Use Logistic Regression predict method to, find more information here in | ||
# [BigFrames](/bigframes/latest/bigframes.ml.linear_model.LogisticRegression#bigframes_ml_linear_model_LogisticRegression_predict) |
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.
Would it result in a clickable link leading to docs.google.com documentation? Asking because in the other place (line 157) we are using absolute https://... path
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 will not. We should use absolute path here in comments.
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.
yes, corrected.
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.
FYI: If this has been corrected, your change hasn't been pushed to GitHub yet.
|
||
predictions = model.predict(features) | ||
|
||
visitor_id = predictions.groupby(["country"])[["predicted_transactions"]].sum() |
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.
Not sure why we call it visitor_id
here, looks same as countries
few lines above
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.
There is another query that asks for predictions for visitors, so the query looks almost identical except for that one change.
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's call SUM(predicted_label) as total_predicted_purchases
in SQL. https://cloud.google.com/bigquery/docs/create-machine-learning-model#run_the_mlpredict_query Let's use the same name as SQL.
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.
yes, will do that.
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.
Yes, will correct that.
|
||
operatingSystem = df["device"].struct.field("operatingSystem") | ||
operatingSystem = operatingSystem.fillna("") | ||
isMobile = df["device"].struct.field("isMobile") |
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.
Let's use snake_case
for this variable.
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.
yes, will correct that.
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.
Do you want me to change that variable name for the earlier part of the code?
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.
Yes, please.
"os": operatingSystem, | ||
"is_mobile": isMobile, | ||
"os": operating_system, | ||
"isMobile": is_mobile, |
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.
The "is_mobile"
string didn't need to be changed, but if you do you must change it everywhere.
@@ -151,7 +143,7 @@ def test_bqml_getting_started(random_model_id): | |||
# - log_loss — The loss function used in a logistic regression. This is the measure of how far the | |||
# model's predictions are from the correct labels. | |||
|
|||
# - roc_auc — The area under the ROC curve. This is the probability that a classifier is more confident that | |||
# - roc_auc — The area under the ROC curve. This is the probability that a classifier is morepy confident that |
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.
Typo: morepy
# - roc_auc — The area under the ROC curve. This is the probability that a classifier is morepy confident that | |
# - roc_auc — The area under the ROC curve. This is the probability that a classifier is more confident that |
"pageviews": pageviews, | ||
} | ||
) | ||
# Use Logistic Regression predict method to, find more information here in |
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.
Incomplete sentence.
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕