Run mypy (and pytype?) on code samples

[tswast]

So that we have some assurance that our customers can use mypy / pytype successfully from our library, I think we should run mypy / pytype on them as well.

Ideally we'd do this from the Samples Kokoro jobs, as that'll be more similar to the development environment that a customer has setup.

Step 0.

• Add type hints to our code samples. (Maybe include in our root mypy/pytype noxfile config?)

Step 1.

• Add optional pytype / mypy sessions to https://github.com/googleapis/synthtool/blob/master/synthtool/gcp/templates/python_samples/noxfile.py.j2

Step 2.

• Enable this feature in our samples, probably via the noxfile_config.py files.

[tswast]

@plamut I kinda consider this a v3 blocker. I'd really like to make sure our type hints are working from the end-user perspective before we start publishing them.

[plamut]

@tswast OK, I can start working on this as soon as I'm done with a similar task in Pub/Sub. It's quite tedious there due to a much more dynamic nature of the code and metaprogramming hacks, but I beleive I'll be done soon.

[tswast]

TIL, there is a enforce_type_hints flag already https://github.com/GoogleCloudPlatform/python-docs-samples/blob/14533af57081783f3671fa6ac0386fea5f6ebc7f/noxfile_config.py#L26-L28

But it only checks that annotations are there, not that they actually work.

[plamut]

But it only checks that annotations are there, not that they actually work.

LGTM! 🙃 😁

[plamut]

Random observation: We need to annotate return types of our __init__() methods.

For example, when checking the samples with more strict options, the following innocent line resulted in a mypy error:

table = bigquery.Table(table_id)

Error:

... Call to untyped function "Table" in typed context

The following seems to be required to appease mypy:

diff --git google/cloud/bigquery/table.py google/cloud/bigquery/table.py
index f434688..5b96369 100644
--- google/cloud/bigquery/table.py
+++ google/cloud/bigquery/table.py
@@ -369,7 +369,7 @@ class Table(_TableBase):
         "require_partition_filter": "requirePartitionFilter",
     }
 
-    def __init__(self, table_ref, schema=None):
+    def __init__(self, table_ref, schema=None) -> "Table":
         table_ref = _table_arg_to_table_ref(table_ref)
         self._properties = {"tableReference": table_ref.to_api_repr(), "labels": {}}
         # Let the @property do validation.

I will either do it in the scope of this issue, or in a separate PR if the required changes turn out to be non-trivial.

[tswast]

We need to annotate return types of our init() methods.

Good catch, thanks! A separate PR sounds good to me, since we know the samples changes will at the very least be quite a few lines of code.

[plamut]

On the other hand... __init__() is technically an initializer that doesn't return anything, it's the __new__() method that actually creates a new instance.

If I apply the workaround suggested above, the mypy checks pass when run through a noxfile located in samples/subfolder. However, if I run the mypy session from the top level noxfile (slightly modified to include samples/), mypy (rightfully) complains that the return type of __init__ should be None.

I need to figure out why this is the case. Annotating __init__() return type like that is a smell, even though it briefly seemed like a decent workaround. It might have to do with how bigquery is installed in the corresponding nox session, it's weird that bigquery.Table is "recognized" as a function. 😕

[tswast]

it's weird that bigquery.Table is "recognized" as a function.

I wonder if we'll need to update our samples to use the original location (e.g. google.cloud.bigquery.table.Table) in order to make type checkers happy? That'd be a bit unfortunate, but would at least align with how our reference docs are organized.

[plamut]

it's weird that bigquery.Table is "recognized" as a function.

I wonder if we'll need to update our samples to use the original location (e.g. google.cloud.bigquery.table.Table) in order to make type checkers happy? That'd be a bit unfortunate, but would at least align with how our reference docs are organized.

It turned out that the fix was to explicitly annotate the return type of __init__() to None, which made it a typed method... meaning that it can be called in typed context.

The error message mentioning "function" was initially somewhat confusing, but I later discovered the true cause of the complaint. It seems that the mentioned change will not be needed.

[tswast]

Fixed by https://github.com/googleapis/python-bigquery/pull/1081/files#diff-f7a16a65f061822bcc73b8296f4dc837353d379d8d9cc5307982cb6941442835R46 which adds the mypy_samples session to the default set of sessions (so it runs as part of the default "kokoro" job.