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.

Sign up for GitHub

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
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions noxfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ def unit_noextras(session):
@nox.session(python=DEFAULT_PYTHON_VERSION)
def pytype(session):
"""Run type checks."""
# 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 •
edited
Loading

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.

session.install("-e", ".[all]")
session.install("ipython")
session.install(PYTYPE_VERSION)
Expand Down