Moved the imports and region tags inside the functions #1891

happyhuman · 2018-12-03T22:41:42Z

Based on the new standard, the imports and the region tags should be placed inside the function.

happyhuman · 2018-12-03T23:10:52Z

@jmdobry @dizcology @alixhami @nnegrey can you please review this PR?

alixhami · 2018-12-04T17:32:26Z

language/cloud-client/v1/snippets.py

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

+    # TODO(developer): Uncomment the following line to run this code.
+    # text = 'Your text to analyze, e.g. Hello, world!'


It would be better to remove the parameter and have the actual text defined here. This will reduce confusion for the user. Only have commented out variables when absolutely necessary (ex. a project ID).

So, the function takes no input parameters as text, and we uncomment text to something like this:
text = 'Hello World'

Yeah?

Yes, you are correct on the 'Hello World' example. For the gcs_uri parameter, upload the sample file to the cloud-samples-data storage bucket, and then update to where you uploaded it. For example: gcs_uri = 'gs://cloud-samples-data/language/your-sample-file.txt'

Did we finalize that new sample rubric yet?

This page the example page alix? https://cloud.google.com/natural-language/docs/quickstart-client-libraries

It's not officially finalized because we're still waiting on UX testing for the script vs. function formatting. I believe eliminating the use of commented out variables when possible was non-controversial, though.

I don't think that link is the "official" example page, though those look like good samples to me.

alixhami · 2018-12-04T17:35:00Z

language/cloud-client/v1/snippets.py

    """Detects sentiment in the file located in Google Cloud Storage."""
    client = language.LanguageServiceClient()

+    # TODO(developer): Uncomment the following line to run this code.
+    # gcs_uri = 'The gcs uri to your file, e.g. gs://my_bucket/my_file'


This URI could be defined here if you put the file in our sample data GCS bucket - gs://cloud-samples-data. This way the user has an easily accessible test file and the snippet tests don't have to upload a file every time. Let me know if you don't have permissions to that project and I'll add you.

alixhami · 2018-12-04T17:35:20Z

language/cloud-client/v1/snippets.py

    """Detects entities in the text."""
    client = language.LanguageServiceClient()

+    # TODO(developer): Uncomment the following line to run this code.
+    # text = 'Your text to analyze, e.g. Hello, world!'


Same as above. Please replace with the actual text.

alixhami · 2018-12-04T17:36:39Z

language/cloud-client/v1/snippets.py

+    import six
+    from google.cloud import language
+    from google.cloud.language import enums
+    from google.cloud.language import types
    """Detects entities in the text."""


Please remove.

alixhami · 2018-12-04T17:37:54Z

language/cloud-client/v1/snippets.py

+    from google.cloud import language
+    from google.cloud.language import enums
+    from google.cloud.language import types
+
    """Detects sentiment in the text."""


Please remove. It doesn't make sense to have a docstring if the user doesn't know it's a function.

alixhami · 2018-12-04T17:42:03Z

language/cloud-client/v1/snippets.py

    """Detects syntax in the file located in Google Cloud Storage."""
    client = language.LanguageServiceClient()

+    # TODO(developer): Uncomment the following line to run this code.
+    # gcs_uri = 'The gcs uri to your file, e.g. gs://my_bucket/my_file'


Same as above. Please upload the file to the sample file bucket.

alixhami · 2018-12-04T17:42:14Z

language/cloud-client/v1/snippets.py

    """Detects entity sentiment in the provided text."""
    client = language.LanguageServiceClient()

+    # TODO(developer): Uncomment the following line to run this code.
+    # text = 'Your text to analyze, e.g. Hello, world!'


Same as above. Please hardcode the text.

alixhami · 2018-12-04T17:42:32Z

language/cloud-client/v1/snippets.py

    """Detects entity sentiment in a Google Cloud Storage file."""
    client = language.LanguageServiceClient()

+    # TODO(developer): Uncomment the following line to run this code.
+    # gcs_uri = 'The gcs uri to your file, e.g. gs://my_bucket/my_file'


Same as above. Please upload the file to the sample bucket.

alixhami · 2018-12-04T17:42:43Z

language/cloud-client/v1/snippets.py

    """Classifies content categories of the provided text."""
    client = language.LanguageServiceClient()

+    # TODO(developer): Uncomment the following line to run this code.
+    # text = 'Your text to analyze, e.g. Hello, world!'


Same as above. Please hardcode the text.

This will need to be longer than 20 tokens or the api call will fail.

alixhami · 2018-12-04T17:43:01Z

language/cloud-client/v1/snippets.py

    """Classifies content categories of the text in a Google Cloud Storage
    file.
    """
    client = language.LanguageServiceClient()

+    # TODO(developer): Uncomment the following line to run this code.
+    # gcs_uri = 'The gcs uri to your file, e.g. gs://my_bucket/my_file'


Same as above. Please upload the file to the sample bucket.

- Sample files no longer have input arguments - Input texts and uri's are hard coded - unit tests are modified accordingly

nnegrey · 2018-12-04T21:52:33Z

language/cloud-client/v1/snippets.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python

-# Copyright 2016 Google, Inc.
+# Copyright 2018 Google, LLC.


For the future, I've been told we only need to update this for new files.
It's just LLC no .

alixhami

Thanks for adding the files to the public bucket! Now they'll be easily accessible for users and other language samples. Just one last python comment.

alixhami · 2018-12-04T22:44:06Z

language/cloud-client/v1/snippets.py

+    from google.cloud.language import enums
+    from google.cloud.language import types
+
+    text = 'Android is a mobile operating system developed by Google, ' + \


The + signs are not necessary here. The string will continue because of the \.

Very interesting. I did not know this little trick. Thanks. :)

…gleCloudPlatform/python-docs-samples#1891) * Moved the imports and region tags inside the functions * Removed the unnecessary imports * Added the missing import (six) to the functions * Removed the extra whitespaces * Changes based on Alix's comments. - Sample files no longer have input arguments - Input texts and uri's are hard coded - unit tests are modified accordingly * Remove extra whitespace * Removed extra whitespaces * Removed unused import * Removed the extra + signs

* Moved the imports and region tags inside the functions * Removed the unnecessary imports * Added the missing import (six) to the functions * Removed the extra whitespaces * Changes based on Alix's comments. - Sample files no longer have input arguments - Input texts and uri's are hard coded - unit tests are modified accordingly * Remove extra whitespace * Removed extra whitespaces * Removed unused import * Removed the extra + signs

…gleCloudPlatform/python-docs-samples#1891) * Moved the imports and region tags inside the functions * Removed the unnecessary imports * Added the missing import (six) to the functions * Removed the extra whitespaces * Changes based on Alix's comments. - Sample files no longer have input arguments - Input texts and uri's are hard coded - unit tests are modified accordingly * Remove extra whitespace * Removed extra whitespaces * Removed unused import * Removed the extra + signs

happyhuman added 2 commits December 3, 2018 14:39

Moved the imports and region tags inside the functions

b0285c8

Removed the unnecessary imports

1008e05

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 3, 2018

happyhuman added 2 commits December 3, 2018 14:53

Added the missing import (six) to the functions

677c3d4

Removed the extra whitespaces

d818474

happyhuman requested review from dizcology, alixhami and jmdobry December 3, 2018 23:05

happyhuman requested a review from nnegrey December 4, 2018 00:06

Merge branch 'master' into master

bc600a5

alixhami requested changes Dec 4, 2018

View reviewed changes

happyhuman and others added 3 commits December 4, 2018 13:42

Changes based on Alix's comments.

3c8e76b

- Sample files no longer have input arguments - Input texts and uri's are hard coded - unit tests are modified accordingly

Remove extra whitespace

9539226

Merge branch 'master' into master

3ec1471

nnegrey reviewed Dec 4, 2018

View reviewed changes

happyhuman added 3 commits December 4, 2018 13:58

Removed extra whitespaces

ed84c48

Merge branch 'master' of github.com:happyhuman/python-docs-samples

a13340d

Removed unused import

1224bad

alixhami reviewed Dec 4, 2018

View reviewed changes

Removed the extra + signs

7e76df7

alixhami approved these changes Dec 4, 2018

View reviewed changes

happyhuman merged commit ce4e7c7 into GoogleCloudPlatform:master Dec 4, 2018

m-strzelczyk mentioned this pull request Nov 18, 2022

migrate code from googleapis/python-language #8533

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved the imports and region tags inside the functions #1891

Moved the imports and region tags inside the functions #1891

happyhuman commented Dec 3, 2018

happyhuman commented Dec 3, 2018 •

edited

Loading

alixhami Dec 4, 2018

happyhuman Dec 4, 2018

alixhami Dec 4, 2018

nnegrey Dec 4, 2018

nnegrey Dec 4, 2018

alixhami Dec 4, 2018

alixhami Dec 4, 2018

alixhami Dec 4, 2018

alixhami Dec 4, 2018

alixhami Dec 4, 2018

alixhami Dec 4, 2018

alixhami Dec 4, 2018

alixhami Dec 4, 2018

alixhami Dec 4, 2018

nnegrey Dec 4, 2018

alixhami Dec 4, 2018

nnegrey Dec 4, 2018

alixhami left a comment

alixhami Dec 4, 2018

happyhuman Dec 4, 2018

Moved the imports and region tags inside the functions #1891

Moved the imports and region tags inside the functions #1891

Conversation

happyhuman commented Dec 3, 2018

happyhuman commented Dec 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alixhami left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

happyhuman commented Dec 3, 2018 •

edited

Loading