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
[BEAM-10708] Added an example notebook for beam_sql magic #15518
Conversation
R: @rohdesamuel |
Codecov Report
@@ Coverage Diff @@
## master #15518 +/- ##
==========================================
- Coverage 83.77% 83.75% -0.02%
==========================================
Files 444 444
Lines 60255 60318 +63
==========================================
+ Hits 50479 50520 +41
- Misses 9776 9798 +22
Continue to review full report at Codecov.
|
Run Python PreCommit |
e96e00a
to
a87a3c8
Compare
max_duration_secs: float, | ||
dynamic_plotting_interval: Optional[int] = None, | ||
include_window_info: bool = False, | ||
display_facets: bool = False) -> None: |
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.
Can you add some comments for these new parameters? I have to look at the implementation to understand their meanings.
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!
"metadata": {}, | ||
"source": [ | ||
"## Query#1 - A simple static query\n", | ||
"`beam_sql` is an IPython [custom magic](https://ipython.readthedocs.io/en/stable/config/custommagics.html). If you're not familiar with magics, here are some [built-in examples](https://ipython.readthedocs.io/en/stable/interactive/magics.html).\n", |
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.
I think this sentence is better placed when you mentioned "magic" above (i.e., first let's load the beam_sql magic), which gives reader more context about magic
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.
I'll move the explanation to the very beginning, immediately after mentioning the beam_sql
magic.
beam_sql
is an IPython custom magic ...
"source": [ | ||
"The `beam_sql` magic shows you the result of the SQL query.\n", | ||
"\n", | ||
"It also creates and outputs a PCollection named `query1_data` with element_type like `BeamSchema_...(id: int32, str: str)`.\n", |
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.
element_type does not display properly.
Please check other name with '_' in this doc
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.
Enclosing literal words with _
with backticks.
"id": "05d2b688", | ||
"metadata": {}, | ||
"source": [ | ||
"To introspect the data again with more knobs, we can use `show`." |
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.
This doc mixes the usage of you and we, sometimes is "you can do something" and sometimes is "we can do something", consider making it consistent.
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.
Change to use you
.
"id": "6faa7df8", | ||
"metadata": {}, | ||
"source": [ | ||
"To materialize the PCollection into a pandas DataFrame object, we can use `collect`." |
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.
Maybe add a link to pandas DataFrame object? I have no idea what is 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.
Got it, adding https://pandas.pydata.org/pandas-docs/stable/user_guide/dsintro.html#dataframe as the link.
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.
Some nits.
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.
LGTM
Run PythonDocker PreCommit |
1. Supported show/collect for beam_sql outputs. 2. Added a parser for beam_sql inputs.
3b09cd4
to
a6bb4ae
Compare
Squashed the commits |
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
ValidatesRunner
compliance status (on master branch)Examples testing status on various runners
Post-Commit SDK/Transform Integration Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.