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

Add small, generated version of language_sentiment_text #1660

Merged
merged 10 commits into from
Aug 30, 2018

Conversation

beccasaurus
Copy link
Contributor

@beccasaurus beccasaurus commented Aug 28, 2018

#language_sentiment_text

This Pull Requests adds our first snippet which has been generated using gapic-generator

Comparison (before & after)

The current, hand-written version of this snippet (language/cloud-client/v1/snippets.py) ::

def sentiment_text(text):
    """Detects sentiment in the text."""
    client = language.LanguageServiceClient()

    if isinstance(text, six.binary_type):
        text = text.decode('utf-8')

    # Instantiates a plain text document.
    document = types.Document(
        content=text,
        type=enums.Document.Type.PLAIN_TEXT)

    # Detects sentiment in the document. You can also analyze HTML with:
    #   document.type == enums.Document.Type.HTML
    sentiment = client.analyze_sentiment(document).document_sentiment

    print('Score: {}'.format(sentiment.score))
    print('Magnitude: {}'.format(sentiment.magnitude))

The new, generated snippet (language/generated-samples/v1/analyze_sentiment.py) ::

Note: I am continuing with the :product/:sample_group/:version/:file.py sample location. Today, most products have a :product/cloud-client/ directory. For the ML APIs, these samples can hopefully all be converted to generated samples and will live in :product/generated-samples/:version/:region_tag.py (for now, future location tbd)

from google.cloud import language_v1
from google.cloud.language_v1 import enums


def sample_analyze_sentiment(content):

    client = language_v1.LanguageServiceClient()

    # content = 'Your text to analyze, e.g. Hello, world!'

    type_ = enums.Document.Type.PLAIN_TEXT
    document = {'type': type_, 'content': content}

    response = client.analyze_sentiment(document)
    sentiment = response.document_sentiment
    print('Score: {}'.format(sentiment.score))
    print('Magnitude: {}'.format(sentiment.magnitude))

✅ Improvements:

  • Imports are shown (fyi: some newer python snippets now show this)
  • Sample Parameter is documented with comment (to date, most Python samples do not include such comments)

🚫 Regressions:

  • type_ is an icky edge-case when currently using certain keywords with the generator. We'll look into correcting this. PLEASE feel free to mark this as a required change to block the sample on this and we'll prioritize looking into it (might not be a trivial change to make)
  • Various extraneous lines of whitespace
    • the double empty lines are a result of running the Python formatter GAPIC uses, will look into further 😕
  • Sample """description""" missing – we will add this back, PLEASE feel free to mark this as a required change to block the sample on this (TRIVIAL change to make)

📝 Sample Source (YAML)

FYI here is the GAPIC configuration for generating the above snippet ::

# RPC Method
- name: AnalyzeSentiment
  # ...

  samples:
    standalone:
    - calling_forms: ".*"
      value_sets: analyze_sentiment
      region_tag: language_sentiment_text

  sample_value_sets:
  - id: analyze_sentiment
    title: "Analyzing Sentiment"
    description: "Proof of concept for analyzing sentiment"
    parameters:
      defaults:
      - document.type=PLAIN_TEXT
      - document.content="Your text to analyze, e.g. Hello, world!"
      attributes:
      - parameter: document.content
        sample_argument: true
    on_success:
    - define: sentiment=$resp.document_sentiment
    - print:
      - "Score: %s"
      - sentiment.score
    - print:
      - "Magnitude: %s"
      - sentiment.magnitude

This will eventually be committed to googleapis/googleapis but at a later date

📖 Instructions for generating this sample

  • 🚧 TODO (later!) 🚧

  • Follow-up PRs with additional generated samples will look at adding instructions to show how y'all can generate these samples. The development setup is very non-trivial so I'm hoping we can avoid this being a blocker for adding this sample 😇

🤔 What about this sample supporting Migration Docs?

The existing language_sentiment_text sample is used in the Migrating to Python Client Library v0.26.1 page

Here are the parts used (part of existing snippet below) ::

def sentiment_text(text):
  # ...

    # Instantiates a plain text document.
    # [START language_python_migration_document_text]      # <---
    # [START language_python_migration_sentiment_text]     # <---
    document = types.Document(
        content=text,
        type=enums.Document.Type.PLAIN_TEXT)
    # [END language_python_migration_document_text]       # <---

    # ...
    print('Magnitude: {}'.format(sentiment.magnitude))
    # [END language_python_migration_sentiment_text]      # <---

The migration regions which are being used to take into account:

  • language_python_migration_document_text

    • This shows how to create a Document type from a simple string (rather than GCS path)
    • Solution: There are other samples in snippets.py which do this so I moved this region tag into another sample
  • language_python_migration_sentiment_text

    • This is specifically for a section of the migration guide which shows "Making an analyze sentiment request and processing the response"
    • Solution: (pending) – instead of this coming from a code sample on GitHub, I'll embed this into the website. The migration pages embed lots of code into the website (which makes sense in the case of migration guides) so (after also running this by a Python co-worker) I'll just do this – AFTER this PR because I'm not deleting the existing code sample in this PR

📝Note about README and CLI

  • Following the latest rubric for How-to Guide code snippets, the generated snippets do not generate either a README or necessarily a CLI as a requirement – today a minimal main() method is included but please do not treat this as a requirement

↪️ Steps

  1. Review & Merge this PR
  2. Website: publish the generated version of the sample
  3. Website: embed the source of language_python_migration_sentiment_text in the website
  4. Send PR to remove the hand-written version of the sample

Rebecca Taylor added 4 commits August 27, 2018 15:54
FYI generated from the following YAML GAPIC config:

  sample_value_sets:
    - id: analyze_sentiment
      title: "Analyzing Sentiment"
      description: "Proof of concept for analyzing sentiment"
      parameters:
        defaults:
        - document.type=PLAIN_TEXT
        - document.content="Your text to analyze, e.g. Hello, world!"
        attributes:
        - parameter: document.content
          sample_argument: true
      on_success:
      - define: sentiment=$resp.document_sentiment
      - print:
        - "Score: %s"
        - sentiment.score
      - print:
        - "Magnitude: %s"
        - sentiment.magnitude
    samples:
      standalone:
      - calling_forms: ".*"
        value_sets: analyze_sentiment
        region_tag: language_sentiment_text
Move language_python_migration_document_text so it uses a different snippet in preparation for deprecation of existing language_sentiment_text sample
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 28, 2018
@@ -39,12 +39,10 @@ def sentiment_text(text):
text = text.decode('utf-8')

# Instantiates a plain text document.
# [START language_python_migration_document_text]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hand-written version of the snippet will be deprecated later.

In preparation, I moved this region tag to a different snippet (below) so it'll be safe to delete this hand-written snippet.

@beccasaurus
Copy link
Contributor Author

beccasaurus commented Aug 28, 2018

Ohhh nooooessss.... the Python linter doesn't like our generated comment:

./language_sentiment_text.py:17:1: E266 too many leading '#' for block comment
#### DO NOT EDIT! This is a generated sample ("Request",  "analyze_sentiment")
^

screen shot 2018-08-28 at 10 11 20 am


# [START language_sentiment_text]

from google.cloud import language_v1
Copy link
Member

Choose a reason for hiding this comment

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

can this be from google.cloud import language_v1 as language?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we also trying to move the imports to inside the code snippet and inside the method?

Not sure what's possible with the auto-gen tool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we also trying to move the imports to inside the code snippet and inside the method

Either! There's a current project to decide on this moving forward. But, that aside, we should prefer imports > no imports. We can put them into the method or outside of the method.

Why do some of the current Python samples have the imports inside the method?

  • because the hand-written snippets are usually in a file alongside many other snippets.py
  • we generate snippets in their own files
  • this means that we can more easily show imports!
  • this ALSO means that we can do this for COMPILED languages as well now!!! 🤘

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'll update this to as language 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dizcology Is this a blocker or can we revisit after this sample?

# [START language_sentiment_text]

from google.cloud import language_v1
from google.cloud.language_v1 import enums
Copy link
Member

Choose a reason for hiding this comment

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

is it possible to not import this separately, but use it as language.enums in the code?


def main():
# FIXME: Convert argv from strings to the correct types.
sample_analyze_sentiment(*sys.argv[1:])
Copy link
Member

Choose a reason for hiding this comment

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

this is okay, but if we are going to keep the command line interface (probably not), it's still better to use argparse if possible.



def sample_analyze_sentiment(content):
# [START language_sentiment_text_core]
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't seen this before, but are the 2 region tags necessary or which one would we use in docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, right! These are here for API Explorer, as it was described to me. I'll look into removing this – it doesn't display for users :)

# content = 'Your text to analyze, e.g. Hello, world!'

type_ = enums.Document.Type.PLAIN_TEXT
document = {'type': type_, 'content': content}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to get the tool to use the helper method here?

document = types.Document(
        content=text,
        type=enums.Document.Type.PLAIN_TEXT)

@nnegrey
Copy link
Contributor

nnegrey commented Aug 30, 2018

All in all, pretty cool and exciting! :)

Copy link
Contributor

@nnegrey nnegrey left a comment

Choose a reason for hiding this comment

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

LGTM
Excited to see this continually developed!!! :)

@beccasaurus beccasaurus merged commit 31aeffb into master Aug 30, 2018
ace-n pushed a commit that referenced this pull request Aug 30, 2018
* Generated sample: language_sentiment_text

FYI generated from the following YAML GAPIC config:

  sample_value_sets:
    - id: analyze_sentiment
      title: "Analyzing Sentiment"
      description: "Proof of concept for analyzing sentiment"
      parameters:
        defaults:
        - document.type=PLAIN_TEXT
        - document.content="Your text to analyze, e.g. Hello, world!"
        attributes:
        - parameter: document.content
          sample_argument: true
      on_success:
      - define: sentiment=$resp.document_sentiment
      - print:
        - "Score: %s"
        - sentiment.score
      - print:
        - "Magnitude: %s"
        - sentiment.magnitude
    samples:
      standalone:
      - calling_forms: ".*"
        value_sets: analyze_sentiment
        region_tag: language_sentiment_text

* Add requirements.txt (not currently generated)

* Add test for language_sentiment_text (not currently generated)

* Move language_python_migration_document_text

Move language_python_migration_document_text so it uses a different snippet in preparation for deprecation of existing language_sentiment_text sample

* Rename generated snippets so filename == region tag

* Fix test for generated code sample (file rename to match region tag)

* Update Copyright year to 2018 in new hand-written file

* Fix lint errors of #language_sentiment_text test

* Regenerate #language_sentiment_text to fix lint errors (updated Python sample template)

* Binary string support in samples!

From PR googleapis/gapic-generator#2272
@alixhami alixhami deleted the OMG-A-GENERATED-SAMPLE-YALL branch September 21, 2018 23:30
busunkim96 pushed a commit to busunkim96/python-language that referenced this pull request May 20, 2020
…oogleCloudPlatform/python-docs-samples#1660)

* Generated sample: language_sentiment_text

FYI generated from the following YAML GAPIC config:

  sample_value_sets:
    - id: analyze_sentiment
      title: "Analyzing Sentiment"
      description: "Proof of concept for analyzing sentiment"
      parameters:
        defaults:
        - document.type=PLAIN_TEXT
        - document.content="Your text to analyze, e.g. Hello, world!"
        attributes:
        - parameter: document.content
          sample_argument: true
      on_success:
      - define: sentiment=$resp.document_sentiment
      - print:
        - "Score: %s"
        - sentiment.score
      - print:
        - "Magnitude: %s"
        - sentiment.magnitude
    samples:
      standalone:
      - calling_forms: ".*"
        value_sets: analyze_sentiment
        region_tag: language_sentiment_text

* Add requirements.txt (not currently generated)

* Add test for language_sentiment_text (not currently generated)

* Move language_python_migration_document_text

Move language_python_migration_document_text so it uses a different snippet in preparation for deprecation of existing language_sentiment_text sample

* Rename generated snippets so filename == region tag

* Fix test for generated code sample (file rename to match region tag)

* Update Copyright year to 2018 in new hand-written file

* Fix lint errors of #language_sentiment_text test

* Regenerate #language_sentiment_text to fix lint errors (updated Python sample template)

* Binary string support in samples!

From PR googleapis/gapic-generator#2272
hkdevandla pushed a commit to hkdevandla/python-language that referenced this pull request Sep 26, 2020
…oogleCloudPlatform/python-docs-samples#1660)

* Generated sample: language_sentiment_text

FYI generated from the following YAML GAPIC config:

  sample_value_sets:
    - id: analyze_sentiment
      title: "Analyzing Sentiment"
      description: "Proof of concept for analyzing sentiment"
      parameters:
        defaults:
        - document.type=PLAIN_TEXT
        - document.content="Your text to analyze, e.g. Hello, world!"
        attributes:
        - parameter: document.content
          sample_argument: true
      on_success:
      - define: sentiment=$resp.document_sentiment
      - print:
        - "Score: %s"
        - sentiment.score
      - print:
        - "Magnitude: %s"
        - sentiment.magnitude
    samples:
      standalone:
      - calling_forms: ".*"
        value_sets: analyze_sentiment
        region_tag: language_sentiment_text

* Add requirements.txt (not currently generated)

* Add test for language_sentiment_text (not currently generated)

* Move language_python_migration_document_text

Move language_python_migration_document_text so it uses a different snippet in preparation for deprecation of existing language_sentiment_text sample

* Rename generated snippets so filename == region tag

* Fix test for generated code sample (file rename to match region tag)

* Update Copyright year to 2018 in new hand-written file

* Fix lint errors of #language_sentiment_text test

* Regenerate #language_sentiment_text to fix lint errors (updated Python sample template)

* Binary string support in samples!

From PR googleapis/gapic-generator#2272
busunkim96 pushed a commit to googleapis/python-language that referenced this pull request Sep 29, 2020
…oogleCloudPlatform/python-docs-samples#1660)

* Generated sample: language_sentiment_text

FYI generated from the following YAML GAPIC config:

  sample_value_sets:
    - id: analyze_sentiment
      title: "Analyzing Sentiment"
      description: "Proof of concept for analyzing sentiment"
      parameters:
        defaults:
        - document.type=PLAIN_TEXT
        - document.content="Your text to analyze, e.g. Hello, world!"
        attributes:
        - parameter: document.content
          sample_argument: true
      on_success:
      - define: sentiment=$resp.document_sentiment
      - print:
        - "Score: %s"
        - sentiment.score
      - print:
        - "Magnitude: %s"
        - sentiment.magnitude
    samples:
      standalone:
      - calling_forms: ".*"
        value_sets: analyze_sentiment
        region_tag: language_sentiment_text

* Add requirements.txt (not currently generated)

* Add test for language_sentiment_text (not currently generated)

* Move language_python_migration_document_text

Move language_python_migration_document_text so it uses a different snippet in preparation for deprecation of existing language_sentiment_text sample

* Rename generated snippets so filename == region tag

* Fix test for generated code sample (file rename to match region tag)

* Update Copyright year to 2018 in new hand-written file

* Fix lint errors of #language_sentiment_text test

* Regenerate #language_sentiment_text to fix lint errors (updated Python sample template)

* Binary string support in samples!

From PR googleapis/gapic-generator#2272
m-strzelczyk pushed a commit that referenced this pull request Nov 18, 2022
…1660)

* Generated sample: language_sentiment_text

FYI generated from the following YAML GAPIC config:

  sample_value_sets:
    - id: analyze_sentiment
      title: "Analyzing Sentiment"
      description: "Proof of concept for analyzing sentiment"
      parameters:
        defaults:
        - document.type=PLAIN_TEXT
        - document.content="Your text to analyze, e.g. Hello, world!"
        attributes:
        - parameter: document.content
          sample_argument: true
      on_success:
      - define: sentiment=$resp.document_sentiment
      - print:
        - "Score: %s"
        - sentiment.score
      - print:
        - "Magnitude: %s"
        - sentiment.magnitude
    samples:
      standalone:
      - calling_forms: ".*"
        value_sets: analyze_sentiment
        region_tag: language_sentiment_text

* Add requirements.txt (not currently generated)

* Add test for language_sentiment_text (not currently generated)

* Move language_python_migration_document_text

Move language_python_migration_document_text so it uses a different snippet in preparation for deprecation of existing language_sentiment_text sample

* Rename generated snippets so filename == region tag

* Fix test for generated code sample (file rename to match region tag)

* Update Copyright year to 2018 in new hand-written file

* Fix lint errors of #language_sentiment_text test

* Regenerate #language_sentiment_text to fix lint errors (updated Python sample template)

* Binary string support in samples!

From PR googleapis/gapic-generator#2272
parthea pushed a commit to googleapis/google-cloud-python that referenced this pull request Jul 6, 2023
…oogleCloudPlatform/python-docs-samples#1660)

* Generated sample: language_sentiment_text

FYI generated from the following YAML GAPIC config:

  sample_value_sets:
    - id: analyze_sentiment
      title: "Analyzing Sentiment"
      description: "Proof of concept for analyzing sentiment"
      parameters:
        defaults:
        - document.type=PLAIN_TEXT
        - document.content="Your text to analyze, e.g. Hello, world!"
        attributes:
        - parameter: document.content
          sample_argument: true
      on_success:
      - define: sentiment=$resp.document_sentiment
      - print:
        - "Score: %s"
        - sentiment.score
      - print:
        - "Magnitude: %s"
        - sentiment.magnitude
    samples:
      standalone:
      - calling_forms: ".*"
        value_sets: analyze_sentiment
        region_tag: language_sentiment_text

* Add requirements.txt (not currently generated)

* Add test for language_sentiment_text (not currently generated)

* Move language_python_migration_document_text

Move language_python_migration_document_text so it uses a different snippet in preparation for deprecation of existing language_sentiment_text sample

* Rename generated snippets so filename == region tag

* Fix test for generated code sample (file rename to match region tag)

* Update Copyright year to 2018 in new hand-written file

* Fix lint errors of #language_sentiment_text test

* Regenerate #language_sentiment_text to fix lint errors (updated Python sample template)

* Binary string support in samples!

From PR googleapis/gapic-generator#2272
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.

4 participants