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

process: add mypy-valid type annotations to samples #1081

Merged
merged 27 commits into from
Dec 14, 2021

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Dec 8, 2021

Towards #1074.

This PR adds type annotations to samples and fixes quite a few issues discovered along. It also adds a new nox session to run type checks with mypy on samples.

The mypy.ini files in samples/* subdirectories are added for the upcoming type-check sessions that will be added to the noxfiles in these directories (through the code generator).

PR checklist:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

plamut and others added 16 commits November 16, 2021 22:57
* feat: allow cell magic body to be a $variable

* Fix missing indefinitive article in error msg

* Adjust test assertion to error message change

* Refactor logic for extracting query variable

* Explicitly warn about missing query variable name

* Thest the query "variable" is not identifier case
…eapis#1073)

* feat: promote `to_arrow_iterable` to public method

* use correct version number

* Update google/cloud/bigquery/table.py

Co-authored-by: Tim Swast <swast@google.com>
* fix: apply timeout to all resumable upload requests

* Fix stub in test case

* Improve timeout type and other type annotations

* Annnotate return type of _do_resumable_upload()
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
…taFrame (googleapis#1078)

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-bigquery/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [ ] Code coverage does not decrease (if any source code was changed)
- [ ] Appropriate docs were updated (if necessary)

Towards #1076 🦕
(edit: moved to googleapis/python-db-dtypes-pandas#45 )
* revoke dataset access setup

* basic template for sample

* sample + test

* revoke dataset access sample

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* docs: add sample for revoking dataset access - update year and string formatting

* docs: add sample for revoking dataset access - move to snippets and change parameter pattern for readibility

* moving update_dataset to /snippets and adjusting imports on both revoke_access and update_access

* Update samples/snippets/revoke_dataset_access.py

removed nested START/END tags

Co-authored-by: Tim Swast <swast@google.com>

* Update samples/snippets/revoke_dataset_access.py

update readability in API request

Co-authored-by: Tim Swast <swast@google.com>

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* updated test

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

* change after running test

* resolving linting failure, rewriting test

* removed relative import errors

* remove relative mport from update_dataset_access

* adding fixture to conftest.py

* updated sample

* updating sample to match new update_access sample

* fixing region tags

* consolidated tests into one file for both methods

* updating test to full_dataset format

* updated revoke sample

* updating test

* refactored sample

* Update samples/snippets/conftest.py

* Update samples/snippets/revoke_dataset_access.py

Co-authored-by: Tim Swast <swast@google.com>

* Update samples/snippets/update_dataset_access.py

Co-authored-by: Tim Swast <swast@google.com>

* Update samples/snippets/revoke_dataset_access.py

Co-authored-by: Tim Swast <swast@google.com>

* Update samples/snippets/revoke_dataset_access.py

Co-authored-by: Tim Swast <swast@google.com>

* refactoring entry

* added comment for entry access

* Update samples/snippets/README.rst

Co-authored-by: Tim Swast <swast@google.com>

* Update samples/snippets/dataset_access_test.py

Co-authored-by: Tim Swast <swast@google.com>

* Update samples/snippets/dataset_access_test.py

Co-authored-by: Tim Swast <swast@google.com>

* added develper TODO in sample

* add comments to samples

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Tim Swast <swast@google.com>
Co-authored-by: Peter Lamut <plamut@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: meredithslota <meredithslota@google.com>
Samples use the code from the core library, but mypy complains about
calling an untyped function in a typed context, thus some additional
type annotations need to be added.
@plamut plamut requested review from tswast and a team December 8, 2021 15:34
@plamut plamut requested review from a team as code owners December 8, 2021 15:34
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Dec 8, 2021
@plamut plamut changed the title Iss 1074 Add mypy-valid type annotations to samples Dec 8, 2021
@plamut plamut changed the title Add mypy-valid type annotations to samples process: add mypy-valid type annotations to samples Dec 8, 2021
@plamut
Copy link
Contributor Author

plamut commented Dec 8, 2021

ImportError: cannot import name 'TypedDict'

This was only added to typing in Python 3.8. I actually fixed this, but not at all places, and I was only running the checks with Python 3.8, missing this. 🙃

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Dec 9, 2021
Copy link
Contributor

@tswast tswast left a comment

Choose a reason for hiding this comment

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

So cool!

A few nits. Plus I'd to see some improvements to the ExternalConfig class to see if we can avoid casts in those samples.

samples/geography/insert_geojson.py Outdated Show resolved Hide resolved
samples/geography/insert_wkt.py Outdated Show resolved Hide resolved
samples/query_external_gcs_temporary_table.py Outdated Show resolved Hide resolved
samples/query_external_sheets_permanent_table.py Outdated Show resolved Hide resolved
samples/query_external_sheets_temporary_table.py Outdated Show resolved Hide resolved
samples/snippets/update_with_dml.py Outdated Show resolved Hide resolved
@snippet-bot
Copy link

snippet-bot bot commented Dec 14, 2021

Here is the summary of changes.

You are about to add 4 region tags.

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

@plamut
Copy link
Contributor Author

plamut commented Dec 14, 2021

@tswast Since there was a new sample added to main, I merged it into this branch. Had to resolve some conflicts, thus that commit should also be checked.

The rest of the comments have been addressed, and I got rid of several cast() calls by using explicit is None assertions instead - let me know how that looks to you.

(and I didn't commit suggestions through the web UI, as that has lead to the CLA bot complaining without merit)

Copy link
Contributor

@tswast tswast left a comment

Choose a reason for hiding this comment

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

Awesome!

It's a bit funny to have assertions in the code samples, but not too out of place. It looks more natural than the type casts.

@plamut
Copy link
Contributor Author

plamut commented Dec 14, 2021

It's a bit funny to have assertions in the code samples, but not too out of place. It looks more natural than the type casts.

True that, but I agree that's much better than those rather ugly casts.

@plamut plamut added the automerge Merge the pull request once unit tests and other checks pass. label Dec 14, 2021
@tswast tswast merged commit 7e3721e into googleapis:v3 Dec 14, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Dec 14, 2021
@plamut plamut deleted the iss-1074 branch December 14, 2021 16:07
tswast added a commit that referenced this pull request Mar 29, 2022
deps!: BigQuery Storage and pyarrow are required dependencies (#776)

fix!: use nullable `Int64` and `boolean` dtypes in `to_dataframe` (#786) 

feat!: destination tables are no-longer removed by `create_job` (#891)

feat!: In `to_dataframe`, use `dbdate` and `dbtime` dtypes from db-dtypes package for BigQuery DATE and TIME columns (#972)

fix!: automatically convert out-of-bounds dates in `to_dataframe`, remove `date_as_object` argument (#972)

feat!: mark the package as type-checked (#1058)

feat!: default to DATETIME type when loading timezone-naive datetimes from Pandas (#1061)

feat: add `api_method` parameter to `Client.query` to select `INSERT` or `QUERY` API (#967)

fix: improve type annotations for mypy validation (#1081)

feat: use `StandardSqlField` class for `Model.feature_columns` and `Model.label_columns` (#1117)

docs: Add migration guide from version 2.x to 3.x (#1027)

Release-As: 3.0.0
waltaskew pushed a commit to waltaskew/python-bigquery that referenced this pull request Jul 20, 2022
deps!: BigQuery Storage and pyarrow are required dependencies (googleapis#776)

fix!: use nullable `Int64` and `boolean` dtypes in `to_dataframe` (googleapis#786) 

feat!: destination tables are no-longer removed by `create_job` (googleapis#891)

feat!: In `to_dataframe`, use `dbdate` and `dbtime` dtypes from db-dtypes package for BigQuery DATE and TIME columns (googleapis#972)

fix!: automatically convert out-of-bounds dates in `to_dataframe`, remove `date_as_object` argument (googleapis#972)

feat!: mark the package as type-checked (googleapis#1058)

feat!: default to DATETIME type when loading timezone-naive datetimes from Pandas (googleapis#1061)

feat: add `api_method` parameter to `Client.query` to select `INSERT` or `QUERY` API (googleapis#967)

fix: improve type annotations for mypy validation (googleapis#1081)

feat: use `StandardSqlField` class for `Model.feature_columns` and `Model.label_columns` (googleapis#1117)

docs: Add migration guide from version 2.x to 3.x (googleapis#1027)

Release-As: 3.0.0
abdelmegahedgoogle pushed a commit to abdelmegahedgoogle/python-bigquery that referenced this pull request Apr 17, 2023
deps!: BigQuery Storage and pyarrow are required dependencies (googleapis#776)

fix!: use nullable `Int64` and `boolean` dtypes in `to_dataframe` (googleapis#786) 

feat!: destination tables are no-longer removed by `create_job` (googleapis#891)

feat!: In `to_dataframe`, use `dbdate` and `dbtime` dtypes from db-dtypes package for BigQuery DATE and TIME columns (googleapis#972)

fix!: automatically convert out-of-bounds dates in `to_dataframe`, remove `date_as_object` argument (googleapis#972)

feat!: mark the package as type-checked (googleapis#1058)

feat!: default to DATETIME type when loading timezone-naive datetimes from Pandas (googleapis#1061)

feat: add `api_method` parameter to `Client.query` to select `INSERT` or `QUERY` API (googleapis#967)

fix: improve type annotations for mypy validation (googleapis#1081)

feat: use `StandardSqlField` class for `Model.feature_columns` and `Model.label_columns` (googleapis#1117)

docs: Add migration guide from version 2.x to 3.x (googleapis#1027)

Release-As: 3.0.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants