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

Refactored the Sentiment Analysis tutorial to use the Cloud Client Lib #713

Merged
merged 1 commit into from
Dec 15, 2016

Conversation

jmdobry
Copy link
Member

@jmdobry jmdobry commented Dec 12, 2016

@jmdobry jmdobry added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 12, 2016
@jmdobry jmdobry requested a review from theacodes December 12, 2016 22:42
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 12, 2016
@theacodes theacodes requested a review from puneith December 12, 2016 22:47
Copy link
Contributor

@theacodes theacodes left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like Puneith to take a look.

@jmdobry
Copy link
Member Author

jmdobry commented Dec 13, 2016

I think you mean @puneith

@jmdobry jmdobry self-assigned this Dec 13, 2016
@jmdobry jmdobry removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 14, 2016
@jmdobry
Copy link
Member Author

jmdobry commented Dec 14, 2016

This is unblocked and ready for final review

@jmdobry jmdobry requested a review from gguuss December 14, 2016 23:28

for i, sentence in enumerate(response['sentences']):
sentence_sentiment = sentence['sentiment']['score']
for i, sentence in enumerate(annotations.sentences):
Copy link
Contributor

Choose a reason for hiding this comment

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

change the variable name from "i" to "index"

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@puneith
Copy link
Contributor

puneith commented Dec 15, 2016

Other than the earlier comment LGTM

@theacodes
Copy link
Contributor

theacodes commented Dec 15, 2016 via email

@gguuss
Copy link
Contributor

gguuss commented Dec 15, 2016

FYI, it looks like flake8 is not liking a couple things:

nox > virtualenv /home/travis/build/GoogleCloudPlatform/python-docs-samples/.nox/lint-sample-language-sentiment
nox > pip install --upgrade flake8 flake8-import-order
nox > chdir ./language/sentiment
nox > flake8 --show-source --builtin gettext --max-complexity 15 --import-order-style google --exclude .nox,.cache,env,lib,generated_pb2 --application-import-names resources,sentiment_analysis_test,sentiment_analysis .
./sentiment_analysis.py:23:1: E302 expected 2 blank lines, found 1
def print_result(annotations):
^
./sentiment_analysis.py:51:13: E128 continuation line under-indented for visual indent
            include_syntax=False, include_entities=False)

@theacodes theacodes merged commit ca7a29b into master Dec 15, 2016
@theacodes theacodes deleted the sentiment-analysis branch December 15, 2016 19:05
hkdevandla pushed a commit to hkdevandla/python-language that referenced this pull request Sep 26, 2020
busunkim96 pushed a commit to googleapis/python-language that referenced this pull request Sep 29, 2020
parthea pushed a commit to googleapis/google-cloud-python that referenced this pull request Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants