Skip to content

chore: avoid pytype error caused by attrs==21.1.0 #656

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

Merged
merged 1 commit into from
May 7, 2021

Conversation

plamut
Copy link
Contributor

@plamut plamut commented May 7, 2021

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)

Fixes #655.

@plamut plamut requested review from tseaver and a team May 7, 2021 08:06
@plamut plamut requested a review from a team as a code owner May 7, 2021 08:06
@plamut plamut requested review from stephaniewang526 and removed request for a team May 7, 2021 08:06
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 7, 2021
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label May 7, 2021
# An indirect dependecy attrs==21.1.0 breaks the check, and installing a less
# recent version avoids the error until a possibly better fix is found.
# https://github.com/googleapis/python-bigquery/issues/655
session.install("attrs==20.3.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this also be set in setup.py? Presumably with <=?

Copy link
Contributor

Choose a reason for hiding this comment

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

For that matter, if it was set there, then we wouldn't need the setting here.

Copy link
Contributor Author

@plamut plamut May 7, 2021

Choose a reason for hiding this comment

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

Since attrs has quite a user base and is thoroughly tested, the error is most likely a false positive by pytype, thus I wouldn't artificially restrict the supported version range.

This workaround is primarily to unblock #653, which fixes the current issue with autosynth PRs (re-formatting the code with a different style, causing the CI checks to fail + noise changes).

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. I'd love to get a knowledge dump from you on pytype. :)

AFAIK, our code doesn't use attr directly.

It seems like you should be able to just tell pytype to ignore 3rd-party modules/packages, but I don' see a way to do that. Apparently, pytype can use a configuration file, but they aren't documented AFAICT.

It seems it should be possible to exclude attr using -x, but I haven't figure out how.

pytype is barely documented afaict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that would also be my preference to silence that false positive without monkey-patching the dependency or by silencing all 3rd party modules. I think I'll create an issue to re-visit this workaround in the future.

@plamut plamut merged commit e24d47e into googleapis:master May 7, 2021
@plamut plamut deleted the iss-655 branch May 7, 2021 14:56
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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The release attrs==21.1.0 broke pytype check
2 participants