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

docs: add code samples for Jupyter/IPython magics #1013

Merged
merged 7 commits into from Oct 29, 2021

Conversation

tswast
Copy link
Contributor

@tswast tswast commented Oct 11, 2021

Follow-up to GoogleCloudPlatform/python-docs-samples#6889, which removed a BigQuery magics sample for using query parameters.

Note: jupyter_tutorial_test.py is a copy of what is in the samples/snippets
folder. Once the docs have been updated to point to this new version, we can
remove that copy and remove the Jupyter/IPython depedencencies from
samples/snippets/requirements.txt.

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:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Note: jupyter_tutorial_test.py is a copy of what is in the `samples/snippets`
folder. Once the docs have been updated to point to this new version, we can
remove that copy and remove the Jupyter/IPython depedencencies from
`samples/snippets/requirements.txt`.
@tswast tswast requested review from loferris and a team October 11, 2021 15:38
@tswast tswast requested review from a team as code owners October 11, 2021 15:38
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 11, 2021
@snippet-bot
Copy link

snippet-bot bot commented Oct 11, 2021

Here is the summary of changes.

You are about to add 4 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@product-auto-label product-auto-label bot added api: bigquery Issues related to the googleapis/python-bigquery API. samples Issues that are directly related to samples. labels Oct 11, 2021
@@ -0,0 +1,147 @@
# Copyright 2018 Google Inc. All Rights Reserved.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm aware that this uses the natality dataset, which I think is on our list to migrate away from, but this is just to move the sample into a different directory from here: https://github.com/googleapis/python-bigquery/blob/main/samples/snippets/jupyter_tutorial_test.py

Samples are used here: https://cloud.google.com/bigquery/docs/visualize-jupyter

Possible we'll want to make this a regular notebook instead and put it here: https://github.com/GoogleCloudPlatform/bigquery-notebooks

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can go ahead with existing samples using natality as long as we circle back to it eventually.

I think that this would be a good candidate for a notebook when we have the embedded notebook pipeline set up. (Possibly Q4.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@loferris Perhaps I should remove this file from this PR? I was moving it to this directory mostly so I can cleanup the requirements.txt file in samples/snippets, but if we think we'll migrate this to a proper notebook soon-ish, then I can wait to clean that up until then.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine as is!

Copy link
Contributor

@loferris loferris left a comment

Choose a reason for hiding this comment

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

Overall looks good, would love some context on writing a sample as a test file.

@@ -0,0 +1,147 @@
# Copyright 2018 Google Inc. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can go ahead with existing samples using natality as long as we circle back to it eventually.

I think that this would be a good candidate for a notebook when we have the embedded notebook pipeline set up. (Possibly Q4.)

# limitations under the License.
"""All of the samples used in the Jupyter notebooks tutorial.

Written as a test to save on boilerplate, since this sample has to similate
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a good workaround for this kind of doc, but I would be interested in converting to a notebook as writing a sample as a test seems counterintuitive to maintain and fairly different from other samples of this type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 on eventually converting to a notebook and integrating into a notebook->docs pipeline

@tswast tswast requested a review from loferris October 14, 2021 15:09
@tswast
Copy link
Contributor Author

tswast commented Oct 15, 2021

@loferris Could you mark this as Approved if there are no other changes I should make?

Copy link
Contributor

@loferris loferris left a comment

Choose a reason for hiding this comment

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

LGTM

@tswast tswast added the automerge Merge the pull request once unit tests and other checks pass. label Oct 29, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit 61141ee into googleapis:main Oct 29, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Oct 29, 2021
abdelmegahedgoogle pushed a commit to abdelmegahedgoogle/python-bigquery that referenced this pull request Apr 17, 2023
Follow-up to GoogleCloudPlatform/python-docs-samples#6889, which removed a BigQuery magics sample for using query parameters.

Note: jupyter_tutorial_test.py is a copy of what is in the `samples/snippets`
folder. Once the docs have been updated to point to this new version, we can
remove that copy and remove the Jupyter/IPython depedencencies from
`samples/snippets/requirements.txt`.

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:
- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-bigquery/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [ ] Code coverage does not decrease (if any source code was changed)
- [ ] Appropriate docs were updated (if necessary)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants