fixit: Add type-hints to Python sample at functions/ocr/app #9990

m-strzelczyk · 2023-05-17T14:05:46Z

Description

Fixes b/280879772

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

I have followed Sample Guidelines from AUTHORING_GUIDE.MD
README is updated to include all relevant information
Tests pass: nox -s py-3.9 (see Test Environment Setup)
Lint pass: nox -s lint (see Test Environment Setup)
These samples need a new API enabled in testing projects to pass (let us know which ones)
These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
Please merge this PR for me once it is approved

kweinmeister · 2023-05-17T15:54:27Z

LGTM (after resolving conflicts). Also there are extra blank lines, which are up to you whether to fix before merging.

kweinmeister · 2023-05-17T15:55:22Z

functions/ocr/app/main.py

@@ -75,7 +89,18 @@ def detect_text(bucket, filename):


 # [START message_validatation_helper]
-def validate_message(message, param):
+def validate_message(message: dict, param: str) -> Any:


Just to confirm, we don't know the type of message['param']?

With this definition, I can't really make any assumptions about message object and what's inside it, so I'm afraid not. Unless we put better limitations and say message: dict[str, str] or something like that - but it's not really obvious from the context that this is the case IMO.

kweinmeister · 2023-05-17T15:55:50Z

functions/ocr/app/main.py

 #
 # Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
+# distributed under the License is distributed on an 'AS IS' BASIS,


Should the linter be changing the license quotes?

It shouldn't. Thanks for noticing.

engelke · 2023-05-18T17:44:01Z

functions/ocr/app/main.py

-def detect_text(bucket, filename):
+def detect_text(bucket: str, filename: str) -> None:
+    """
+    Extract the text from an image uploaded to Cloud Storage, then


A one line summary followed by a blank line and then more details is preferred. But it's not required.

gcf-merge-on-green · 2023-05-18T23:48:15Z

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

Co-authored-by: Charles Engelke <engelke@google.com>

b/280879671 ## Description Fixes b/280879671 Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google. ## Checklist - [ ] I have followed [Sample Guidelines from AUTHORING_GUIDE.MD](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md) - [ ] README is updated to include [all relevant information](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#readme-file) - [ ] **Tests** pass: `nox -s py-3.9` (see [Test Environment Setup](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup)) - [ ] **Lint** pass: `nox -s lint` (see [Test Environment Setup](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup)) - [ ] These samples need a new **API enabled** in testing projects to pass (let us know which ones) - [ ] These samples need a new/updated **env vars** in testing projects set to pass (let us know which ones) - [ ] This sample adds a new sample directory, and I updated the [CODEOWNERS file](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/.github/CODEOWNERS) with the codeowners for this sample - [ ] This sample adds a new **Product API**, and I updated the [Blunderbuss issue/PR auto-assigner](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/.github/blunderbuss.yml) with the codeowners for this sample - [ ] Please **merge** this PR for me once it is approved

…ases (#9872) ## Description Implemenetd dlp_inspect_image_listed_infotypes with unit test cases. Java equivalent: https://cloud.google.com/dlp/docs/samples/dlp-inspect-image-listed-infotypes#dlp_inspect_image_listed_infotypes-java Fixes #<ISSUE-NUMBER> Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google. ## Checklist - [X] I have followed [Sample Guidelines from AUTHORING_GUIDE.MD](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md) - [ ] README is updated to include [all relevant information](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#readme-file) - [X] **Tests** pass: `nox -s py-3.9` (see [Test Environment Setup](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup)) - [X] **Lint** pass: `nox -s lint` (see [Test Environment Setup](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup)) - [ ] These samples need a new **API enabled** in testing projects to pass (let us know which ones) - [ ] These samples need a new/updated **env vars** in testing projects set to pass (let us know which ones) - [ ] This sample adds a new sample directory, and I updated the [CODEOWNERS file](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/.github/CODEOWNERS) with the codeowners for this sample - [ ] This sample adds a new **Product API**, and I updated the [Blunderbuss issue/PR auto-assigner](https://togithub.com/GoogleCloudPlatform/python-docs-samples/blob/main/.github/blunderbuss.yml) with the codeowners for this sample - [X] Please **merge** this PR for me once it is approved

Co-authored-by: Charles Engelke <engelke@google.com>

* chore: update docstrings for Cloud Run samples * lint * Update render.py

* fix: Add type hints for KMS snippets * fix: type * fix: even more types * black * fix: remove debugging note * isort * manual sort, apparently * Move all imports to top of files, sort, move region tags * even more import fixes * black, isort * black, again

snippet-bot · 2023-05-19T15:03:22Z

No region tags are edited in this PR.

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

engelke

I think those changes will make the failing test pass.

engelke · 2023-05-23T20:53:20Z

functions/ocr/app/main.py

@@ -16,6 +16,7 @@
 import base64
 import json
 import os
+from typing import TypedDict, TypeVar


Change to from typing import Any

engelke · 2023-05-23T20:53:34Z

functions/ocr/app/main.py

@@ -74,9 +89,22 @@ def detect_text(bucket, filename):

 # [END functions_ocr_detect]

+T = TypeVar('T')


engelke · 2023-05-23T20:54:13Z

functions/ocr/app/main.py


 # [START message_validatation_helper]
-def validate_message(message, param):
+def validate_message(message: TypedDict[str, T], param: str) -> T:


Change to def validate_message(message: dict[str, Any], param: str) -> Any:

…79772

m-strzelczyk · 2023-05-24T11:41:34Z

I think those changes will make the failing test pass.

I tried the Any type before. Linter doesn't like it: https://source.cloud.google.com/results/invocations/b6ba4226-f2bc-4547-99bd-d2cc3f9f4c4c/targets/cloud-devrel%2Fpython-docs-samples%2Flint%2Fpresubmit/log

However, I think I found a working typing definition now. All that's left is this CLA check - I think we should ignore it? The commit that triggers it came from a merge with origin/main, so it is actually already a part of the repo.

fixit: Add type-hints to Python sample at functions/ocr/app

dfd91b2

product-auto-label bot added samples Issues that are directly related to samples. api: cloudfunctions Issues related to the Cloud Run functions API. labels May 17, 2023

m-strzelczyk added 2 commits May 17, 2023 16:18

Fixing stuff

bf04e4c

Fixing stuff

3abb25b

m-strzelczyk marked this pull request as ready for review May 17, 2023 14:45

m-strzelczyk requested review from a team as code owners May 17, 2023 14:45

kweinmeister approved these changes May 17, 2023

View reviewed changes

kweinmeister reviewed May 17, 2023

View reviewed changes

m-strzelczyk added 2 commits May 18, 2023 16:29

Enforcing type checks

db4cb13

Enforcing type checks

3bccdde

engelke approved these changes May 18, 2023

View reviewed changes

Merge branch 'main' into strzelczyk/fixit/280879772

d2f977f

engelke added the automerge Merge the pull request once unit tests and other checks pass. label May 18, 2023

engelke self-assigned this May 18, 2023

gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label May 18, 2023

m-strzelczyk and others added 8 commits May 19, 2023 17:03

Fixing any typing

c7c5b59

chore(deps): update dependency shapely to v2 (#10004)

f40ab30

Adding deletion of rows with NULL dag_id (#10002)

ad8ac5b

Co-authored-by: Charles Engelke <engelke@google.com>

chore(deps): update dependency structlog to v22.3.0 (#8540)

4dba973

Co-authored-by: Charles Engelke <engelke@google.com>

fixit: update docstrings for Cloud Run samples (#10007)

844d6e7

* chore: update docstrings for Cloud Run samples * lint * Update render.py

m-strzelczyk requested a review from a team as a code owner May 19, 2023 15:03

m-strzelczyk requested review from a team as code owners May 19, 2023 15:03

engelke and others added 5 commits May 19, 2023 14:25

Merge branch 'main' into strzelczyk/fixit/280879772

5123217

Merge branch 'main' into strzelczyk/fixit/280879772

82aa941

Merge branch 'main' into strzelczyk/fixit/280879772

447fc3c

Fixing typing info

1fa7bb5

Merge branch 'main' into strzelczyk/fixit/280879772

d35d04a

engelke assigned m-strzelczyk and unassigned engelke May 23, 2023

engelke approved these changes May 23, 2023

View reviewed changes

m-strzelczyk added 2 commits May 24, 2023 13:24

Trying to fix typing

0349e5b

Merge remote-tracking branch 'origin/main' into strzelczyk/fixit/2808…

139cb40

…79772

Merge branch 'main' into strzelczyk/fixit/280879772

71ac232

engelke added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 25, 2023

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 25, 2023

Merge branch 'main' into strzelczyk/fixit/280879772

ae53525

m-strzelczyk merged commit 8b4947d into main May 27, 2023

m-strzelczyk deleted the strzelczyk/fixit/280879772 branch May 27, 2023 19:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixit: Add type-hints to Python sample at functions/ocr/app #9990

fixit: Add type-hints to Python sample at functions/ocr/app #9990

m-strzelczyk commented May 17, 2023

kweinmeister commented May 17, 2023

kweinmeister May 17, 2023

m-strzelczyk May 18, 2023

kweinmeister May 17, 2023

m-strzelczyk May 18, 2023

engelke May 18, 2023

gcf-merge-on-green bot commented May 18, 2023

snippet-bot bot commented May 19, 2023 •

edited

Loading

engelke left a comment

engelke May 23, 2023

engelke May 23, 2023

engelke May 23, 2023

m-strzelczyk commented May 24, 2023

		@@ -74,9 +89,22 @@ def detect_text(bucket, filename):

		# [END functions_ocr_detect]

		T = TypeVar('T')

fixit: Add type-hints to Python sample at functions/ocr/app #9990

fixit: Add type-hints to Python sample at functions/ocr/app #9990

Conversation

m-strzelczyk commented May 17, 2023

Description

Checklist

kweinmeister commented May 17, 2023

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

gcf-merge-on-green bot commented May 18, 2023

snippet-bot bot commented May 19, 2023 • edited Loading

No region tags are edited in this PR.

engelke left a 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

m-strzelczyk commented May 24, 2023

snippet-bot bot commented May 19, 2023 •

edited

Loading