Skip to content
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

Create explore_data_in_bigquery_with_workbench.ipynb #885

Merged
merged 22 commits into from Aug 26, 2022

Conversation

alokpattani
Copy link
Contributor

@alokpattani alokpattani commented Aug 19, 2022

Adding in notebook for exploratory data analysis as part of "Data to AI" effort. See this Colab for what this notebook looks like after it is run: https://colab.research.google.com/drive/1JeNeMtj2A_5P5vo9wxSkrwHQM5JQSoAu. Submitting it with outputs shown since a lot of this about interactive visualization, which can inspire folks to use/read the notebook beyond just the code.

REQUIRED: Fill out the below checklists or remove if irrelevant

  1. If you are opening a PR for Official Notebooks under the notebooks/official folder, follow this mandatory checklist:
  • Use the notebook template as a starting point.
  • Follow the style and grammar rules outlined in the above notebook template.
  • Verify the notebook runs successfully in Colab since the automated tests cannot guarantee this even when it passes.
  • Passes all the required automated checks. You can locally test for formatting and linting with these instructions.
  • You have consulted with a tech writer to see if tech writer review is necessary. If so, the notebook has been reviewed by a tech writer, and they have approved it.
  • This notebook has been added to the CODEOWNERS file under the Official Notebooks section, pointing to the author or the author's team.
  • The Jupyter notebook cleans up any artifacts it has created (datasets, ML models, endpoints, etc) so as not to eat up unnecessary resources.

Adding in notebook for exploratory data analysis as part of "Data to AI" effort. See this Colab for what this notebook looks like after it is run: https://colab.research.google.com/drive/1JeNeMtj2A_5P5vo9wxSkrwHQM5JQSoAu. Submitting it with outputs shown since a lot of this about interactive visualization, which can inspire folks to use/read the notebook beyond just the code.
@alokpattani alokpattani requested a review from a team as a code owner August 19, 2022 00:39
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@soheilazangeneh soheilazangeneh self-assigned this Aug 19, 2022
@review-notebook-app
Copy link

review-notebook-app bot commented Aug 24, 2022

View / edit / reply to this conversation on ReviewNB

ivanmkc commented on 2022-08-24T23:19:39Z
----------------------------------------------------------------

Line #2.    def bq_query(sql, show_job_id=False):

Consider using a verb for your method name, like: run_bq_query.

This is subjective but see https://softwareengineering.stackexchange.com/a/334196 for a discussion.


alokpattani commented on 2022-08-24T23:41:51Z
----------------------------------------------------------------

Acknowledged. Leaving it this time (for consistency - this naming has been used in other notebooks that may get published) but can consider for future functions.

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 24, 2022

View / edit / reply to this conversation on ReviewNB

ivanmkc commented on 2022-08-24T23:19:40Z
----------------------------------------------------------------

Please clean up all relevant BQ resources.


alokpattani commented on 2022-08-24T23:31:48Z
----------------------------------------------------------------

So there actually aren't any BQ resources to clean up since we use a public dataset and don't create any views, tables, models, etc. It's basically just different queries pulling data into notebooks and displaying as tables and charts in here, with outputs to Cloud Storage.

Copy link
Contributor

@ivanmkc ivanmkc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after fixing CODEOWNERS. Can merge after TW's sign-off.

Copy link
Contributor Author

So there actually aren't any BQ resources to clean up since we use a public dataset and don't create any views, tables, models, etc. It's basically just different queries pulling data into notebooks and displaying as tables and charts in here, with outputs to Cloud Storage.


View entire conversation on ReviewNB

Copy link
Contributor Author

Acknowledged. Leaving it this time (for consistency - this naming has been used in other notebooks that may get published) but can consider for future functions.


View entire conversation on ReviewNB

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 25, 2022

View / edit / reply to this conversation on ReviewNB

aarondietz234 commented on 2022-08-25T04:07:16Z
----------------------------------------------------------------

In the Overview:

Nit: This is really picky but I don't like to describe the audience as a target--can we replace "targeted toward" with "written for"?

#######

In the Objective section, in the list of Google Cloud data analytics and ML services:

I'd add Vertex AI, separate from Vertex AI Workbench. They are two different APIs/services, and only Workbench is optional, if I understand this tutorial correctly.

#######

In the Costs section, I'd either remove "(optional)" from Vertex AI (because we still use the SDK even if we don't use Vertex AI Workbench), or split them out again, like in the Objective section:

  • Vertex AI
  • Vertex AI Workbench (optional)

#######

If you make the previous suggested change, consider making the pricing info more clear, by changing "Vertex AI pricing" to "Vertex AI and Vertex AI Workbench pricing". Not sure what's clearer, really, but it may help them understand that Vertex AI and Vertex AI Workbench pricing are on the same page.


alokpattani commented on 2022-08-25T06:34:41Z
----------------------------------------------------------------

Made most/all of these changes. I linked to the Workbench section in the Vertex AI pricing guide separately from the link to the overall Vertex AI page.

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 25, 2022

View / edit / reply to this conversation on ReviewNB

aarondietz234 commented on 2022-08-25T04:07:17Z
----------------------------------------------------------------

Nit: Just a branding suggestion--lowercase "notebooks", here and throughout this tutorial. If this is how it is in the template, disregard and someday I'll take a look at the template and suggest the change.


alokpattani commented on 2022-08-25T06:40:29Z
----------------------------------------------------------------

Went through and changed all "notebook" to lowercase except 1-2 where it goes with Jupyter (not Vertex) as something like a product title or display heading.

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 25, 2022

View / edit / reply to this conversation on ReviewNB

aarondietz234 commented on 2022-08-25T04:07:18Z
----------------------------------------------------------------

Nit: Change "Vertex AI Notebook requires" to:

Vertex AI Workbench notebooks require


alokpattani commented on 2022-08-25T06:40:51Z
----------------------------------------------------------------

Done.

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 25, 2022

View / edit / reply to this conversation on ReviewNB

aarondietz234 commented on 2022-08-25T04:07:19Z
----------------------------------------------------------------

Nit: Remove "off" from "save off some of the outputs..."


alokpattani commented on 2022-08-25T06:41:07Z
----------------------------------------------------------------

Done.

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 25, 2022

View / edit / reply to this conversation on ReviewNB

aarondietz234 commented on 2022-08-25T04:07:19Z
----------------------------------------------------------------

Remove an 's' to change "a sets of dates" to "a set of dates". Or perhaps it should be "sets of dates" without the 'a' but I'm guessing it should be the first option.


alokpattani commented on 2022-08-25T06:41:47Z
----------------------------------------------------------------

Changed to "a set of dates"

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 25, 2022

View / edit / reply to this conversation on ReviewNB

aarondietz234 commented on 2022-08-25T04:07:20Z
----------------------------------------------------------------

Grammar nit: Add an 'in' to "which you looked at the individual event..." to make it:

"which you looked at in the individual event..."


alokpattani commented on 2022-08-25T06:43:32Z
----------------------------------------------------------------

The point I was making was more intricate (adding the "in" wasn't technically correct, but I get why it felt like it could be), so I ended up re-wording a couple parts of this paragraph altogether - I think it flows better now.

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 25, 2022

View / edit / reply to this conversation on ReviewNB

aarondietz234 commented on 2022-08-25T04:07:21Z
----------------------------------------------------------------

Add a space between the first and second sentences.


alokpattani commented on 2022-08-25T06:43:51Z
----------------------------------------------------------------

Done.

Copy link
Contributor Author

Made most/all of these changes.


View entire conversation on ReviewNB

Copy link
Contributor Author

Went through and changed all "notebook" to lowercase except 1-2 where it goes with Jupyter (not Vertex) as something like a product title or display heading.


View entire conversation on ReviewNB

Copy link
Contributor Author

Done.


View entire conversation on ReviewNB

1 similar comment
Copy link
Contributor Author

Done.


View entire conversation on ReviewNB

Copy link
Contributor Author

Changed to "a set of dates"


View entire conversation on ReviewNB

Copy link
Contributor Author

The point I was making was more intricate (adding the "in" wasn't technically correct, but I get why it felt like it could be), so I ended up re-wording a couple parts of this paragraph altogether - I think it flows better now.


View entire conversation on ReviewNB

Copy link
Contributor Author

Done.


View entire conversation on ReviewNB

@andrewferlitsch andrewferlitsch merged commit 89f4571 into GoogleCloudPlatform:main Aug 26, 2022
peterping666 pushed a commit to peterping666/vertex-ai-samples that referenced this pull request Sep 12, 2022
…form#885)

* Create explore_data_in_bigquery_with_workbench.ipynb

Adding in notebook for exploratory data analysis as part of "Data to AI" effort. See this Colab for what this notebook looks like after it is run: https://colab.research.google.com/drive/1JeNeMtj2A_5P5vo9wxSkrwHQM5JQSoAu. Submitting it with outputs shown since a lot of this about interactive visualization, which can inspire folks to use/read the notebook beyond just the code.

* Update CODEOWNERS

Adding owner for forthcoming exploratory data analysis notebook

* Update CODEOWNERS

* Updating exploratory data analysis notebook with latest updates from linter/review

* Updated notebook formatting to try to pass format test

* Trying again to pass notebook formatting test

* Trying again to pass notebook formatting test

* Trying again to pass notebook formatting test

* Linted version of notebook & better project picker

* Uploading linted version from ivanmkc@

* Update CODEOWNERS with EDA notebook

* Updated notebook w/ Tech Writer edits, re-ran all

* 1-2 minor text updates, try to pass linter again

* Trying w/ updated linted file from ivanmkc@

Co-authored-by: Ivan Cheung <ivans.mailbox@gmail.com>
Co-authored-by: Andrew Ferlitsch <aferlitsch@google.com>
@alokpattani alokpattani deleted the patch-2 branch February 22, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants