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

Install 'grpc' deps into 'lint' environ; ignore 'google' imports under pylint #2096

Merged
merged 1 commit into from
Aug 17, 2016
Merged

Install 'grpc' deps into 'lint' environ; ignore 'google' imports under pylint #2096

merged 1 commit into from
Aug 17, 2016

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Aug 11, 2016

Rationale:

pylint tries to guess at imports in the google namespace, and fails miserably. We test that those imports are valid all the time in unit tests anyway, so just disable them in pylint.

@tseaver tseaver self-assigned this Aug 11, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 11, 2016
@tseaver tseaver assigned dhermes and unassigned tseaver Aug 11, 2016
@@ -322,7 +322,7 @@ no-docstring-rgx=__.*__
# DEFAULT: ignored-modules=
# RATIONALE: six aliases stuff for compatibility.
# google.protobuf fixes up namespace package "late".
ignored-modules = six, google.protobuf
ignored-modules = six, google

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Rationale:

pylint tries to guess at imports in the 'google' namespace, and fails
miserably.  We test that those imports are valid all the time in unit tests
anyway, so just disable them in pylint.
@tseaver
Copy link
Contributor Author

tseaver commented Aug 17, 2016

@dhermes I've updated pylintrc_default to disable the redundant and buggy import-error check.

@dhermes
Copy link
Contributor

dhermes commented Aug 17, 2016

the redundant and buggy

Evidence of this? Suggestions for a replacement to lint our imports? (I believe oauth2client uses a flake8 import check plugin.)

@tseaver
Copy link
Contributor Author

tseaver commented Aug 17, 2016

With the import-error suppression commented out:

$ tox -e lint
...
************* Module gcloud.logging._gax
I:562, 0: Locally disabling too-many-branches (R0912) (locally-disabled)
I:608, 0: Locally enabling too-many-branches (R0912) (locally-enabled)
E: 23, 0: Unable to import 'google.logging.type.log_severity_pb2' (import-error)
E: 24, 0: Unable to import 'google.logging.v2.logging_config_pb2' (import-error)
E: 25, 0: Unable to import 'google.logging.v2.logging_metrics_pb2' (import-error)
E: 26, 0: Unable to import 'google.logging.v2.log_entry_pb2' (import-error)
...
************* Module gcloud.pubsub._gax
E: 21, 0: Unable to import 'google.pubsub.v1.pubsub_pb2' (import-error)
E: 22, 0: Unable to import 'google.pubsub.v1.pubsub_pb2' (import-error)

But, without touching the lint environment:

$ .tox/lint/bin/python -c "import google.logging.type.log_severity_pb2, google.logging.v2.logging_config_pb2, google.logging.v2.logging_metrics_pb2, google.logging.v2.log_entry_pb2, google.pubsub.v1.pubsub_pb2; print('OK')"
OK

In any case, it is pointless for them to check imports: we have tests for that.

@tseaver
Copy link
Contributor Author

tseaver commented Aug 17, 2016

Maybe this is the relevant issue?

@dhermes
Copy link
Contributor

dhermes commented Aug 17, 2016

@PCManticore has been a joy to work with on pylint issues. pylint is a static analysis checker lints via static analysis, hence the discrepancy between a working import and the pylint check. It seems we should file a bug with the GAX folks about whatever improper namespace package config is reference in pylint-dev/pylint#687

@tseaver
Copy link
Contributor Author

tseaver commented Aug 17, 2016

@dhermes Why would we care about pylint reporting import errors: we know the imports are good, because 100% test coverage.

@dhermes
Copy link
Contributor

dhermes commented Aug 17, 2016

OK LGTM, go ahead and merge

@tseaver tseaver merged commit b01aa48 into googleapis:master Aug 17, 2016
@tseaver tseaver deleted the pylint-install-grpc-ignore-google-imports branch August 17, 2016 20:32
@PCManticore
Copy link

@dhermes Probably worth reporting an issue on the bug tracker? Sorry for chiming in on a merged PR.

@tseaver
Copy link
Contributor Author

tseaver commented Aug 17, 2016

@PCManticore pylint-dev/pylint#687 looks like the open issue that matches: the various packages which install into the google namespace do not all use the same boilerplate for the namespace's __init__.py. GIven the widesspread adoption of pip/setuptools for installing namespace'd distributions, having the pkg_resources bit work as well as the pkgutils fallback would be ideal.

@dhermes dhermes mentioned this pull request Sep 19, 2016
parthea pushed a commit that referenced this pull request Jun 4, 2023
* Data Labeling Beta samples [(#2096)](GoogleCloudPlatform/python-docs-samples#2096)

* add files

* upate create_annotation_spec_set and test

* add requirements.txt

* update create_instruction and test

* update import data and test

* add label image and test

* add label_text test

* add label_video_test

* add manage dataset and tests

* flake

* fix

* add README

* Adds updates including compute [(#2436)](GoogleCloudPlatform/python-docs-samples#2436)

* Adds updates including compute

* Python 2 compat pytest

* Fixing weird \r\n issue from GH merge

* Put asset tests back in

* Re-add pod operator test

* Hack parameter for k8s pod operator

* Update datalabeling samples to hit test endpoint. [(#2641)](GoogleCloudPlatform/python-docs-samples#2641)

* Auto-update dependencies. [(#2005)](GoogleCloudPlatform/python-docs-samples#2005)

* Auto-update dependencies.

* Revert update of appengine/flexible/datastore.

* revert update of appengine/flexible/scipy

* revert update of bigquery/bqml

* revert update of bigquery/cloud-client

* revert update of bigquery/datalab-migration

* revert update of bigtable/quickstart

* revert update of compute/api

* revert update of container_registry/container_analysis

* revert update of dataflow/run_template

* revert update of datastore/cloud-ndb

* revert update of dialogflow/cloud-client

* revert update of dlp

* revert update of functions/imagemagick

* revert update of functions/ocr/app

* revert update of healthcare/api-client/fhir

* revert update of iam/api-client

* revert update of iot/api-client/gcs_file_to_device

* revert update of iot/api-client/mqtt_example

* revert update of language/automl

* revert update of run/image-processing

* revert update of vision/automl

* revert update testing/requirements.txt

* revert update of vision/cloud-client/detect

* revert update of vision/cloud-client/product_search

* revert update of jobs/v2/api_client

* revert update of jobs/v3/api_client

* revert update of opencensus

* revert update of translate/cloud-client

* revert update to speech/cloud-client

Co-authored-by: Kurtis Van Gent <[email protected]>
Co-authored-by: Doug Mahugh <[email protected]>

* Update datalabeling to match lint. [(#2642)](GoogleCloudPlatform/python-docs-samples#2642)

* datalabeling: ensure all tests use test endpoint [(#2918)](GoogleCloudPlatform/python-docs-samples#2918)

* datalabeling: ensure all tests use test endpoint

* requires an input csv for text input, slight print statement cleanup

Co-authored-by: Leah E. Cole <[email protected]>

* chore(deps): update dependency google-cloud-datalabeling to v0.4.0 [(#3081)](GoogleCloudPlatform/python-docs-samples#3081)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [google-cloud-datalabeling](https://togithub.com/googleapis/python-datalabeling) | minor | `==0.3.0` -> `==0.4.0` |

---

### Release Notes

<details>
<summary>googleapis/python-datalabeling</summary>

### [`v0.4.0`](https://togithub.com/googleapis/python-datalabeling/blob/master/CHANGELOG.md#&#8203;040-httpswwwgithubcomgoogleapispython-datalabelingcomparev030v040-2020-01-31)

[Compare Source](https://togithub.com/googleapis/python-datalabeling/compare/v0.3.0...v0.4.0)

##### Features

-   **datalabeling:** undeprecate resource name helper methods (via synth) ([#&#8203;10039](https://www.github.com/googleapis/python-datalabeling/issues/10039)) ([88f8090](https://www.github.com/googleapis/python-datalabeling/commit/88f809008ee6a709c02c78b1d93af779fab19adb))

##### Bug Fixes

-   **datalabeling:** deprecate resource name helper methods (via synth) ([#&#8203;9832](https://www.github.com/googleapis/python-datalabeling/issues/9832)) ([e5f9021](https://www.github.com/googleapis/python-datalabeling/commit/e5f902154ebe7fcb139aa405babfe9993fd51319))

</details>

---

### Renovate configuration

:date: **Schedule**: At any time (no schedule defined).

:vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

:recycle: **Rebasing**: Never, or you tick the rebase/retry checkbox.

:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#GoogleCloudPlatform/python-docs-samples).

* Simplify noxfile setup. [(#2806)](GoogleCloudPlatform/python-docs-samples#2806)

* chore(deps): update dependency requests to v2.23.0

* Simplify noxfile and add version control.

* Configure appengine/standard to only test Python 2.7.

* Update Kokokro configs to match noxfile.

* Add requirements-test to each folder.

* Remove Py2 versions from everything execept appengine/standard.

* Remove conftest.py.

* Remove appengine/standard/conftest.py

* Remove 'no-sucess-flaky-report' from pytest.ini.

* Add GAE SDK back to appengine/standard tests.

* Fix typo.

* Roll pytest to python 2 version.

* Add a bunch of testing requirements.

* Remove typo.

* Add appengine lib directory back in.

* Add some additional requirements.

* Fix issue with flake8 args.

* Even more requirements.

* Readd appengine conftest.py.

* Add a few more requirements.

* Even more Appengine requirements.

* Add webtest for appengine/standard/mailgun.

* Add some additional requirements.

* Add workaround for issue with mailjet-rest.

* Add responses for appengine/standard/mailjet.

Co-authored-by: Renovate Bot <[email protected]>

* testing: mark some tests as flaky [(#3288)](GoogleCloudPlatform/python-docs-samples#3288)

fixes #3138

* [datalabeling] testing: wrap rpcs with backoff [(#3443)](GoogleCloudPlatform/python-docs-samples#3443)

* wrap all the rpcs with backoff
* add a shared testing lib
* remove flaky

* [datalabeling] fix: clean up old datasets before the test [(#3707)](GoogleCloudPlatform/python-docs-samples#3707)

fixes #3710 
fixes #3711

* [datalabeling] testing: retry upon ServerError [(#3762)](GoogleCloudPlatform/python-docs-samples#3762)

fixes #3760

* Replace GCLOUD_PROJECT with GOOGLE_CLOUD_PROJECT. [(#4022)](GoogleCloudPlatform/python-docs-samples#4022)

* chore(deps): update dependency pytest to v5.4.3 [(#4279)](GoogleCloudPlatform/python-docs-samples#4279)

* chore(deps): update dependency pytest to v5.4.3

* specify pytest for python 2 in appengine

Co-authored-by: Leah Cole <[email protected]>

* Update dependency pytest to v6 [(#4390)](GoogleCloudPlatform/python-docs-samples#4390)

* chore: update templates

* chore: fix docs error

* chore: skip unavailable samples

* chore: use staging endpoint for labeling tests

Co-authored-by: Rebecca Taylor <[email protected]>
Co-authored-by: Gus Class <[email protected]>
Co-authored-by: Kurtis Van Gent <[email protected]>
Co-authored-by: DPEBot <[email protected]>
Co-authored-by: Doug Mahugh <[email protected]>
Co-authored-by: Noah Negrey <[email protected]>
Co-authored-by: Leah E. Cole <[email protected]>
Co-authored-by: WhiteSource Renovate <[email protected]>
Co-authored-by: Takashi Matsuo <[email protected]>
Co-authored-by: Leah Cole <[email protected]>
Co-authored-by: Bu Sun Kim <[email protected]>
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