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

Check changed files with pytype on github actions #3571

Merged
merged 13 commits into from
Oct 22, 2020

Conversation

phoenix-meadowlark
Copy link
Contributor

No description provided.

@phoenix-meadowlark phoenix-meadowlark added infrastructure Relating to build systems, CI, or testing integrations Relating to high-level frontend integrations labels Oct 21, 2020
@google-cla google-cla bot added the cla: yes label Oct 21, 2020
@phoenix-meadowlark phoenix-meadowlark marked this pull request as ready for review October 22, 2020 03:09
@phoenix-meadowlark phoenix-meadowlark changed the title [Test PR] Check changed files with pytype on github actions Check changed files with pytype on github actions Oct 22, 2020
build_tools/pytype/check_diff.sh Show resolved Hide resolved
build_tools/pytype/check_diff.sh Outdated Show resolved Hide resolved
build_tools/pytype/check_diff.sh Outdated Show resolved Hide resolved
build_tools/pytype/check_diff.sh Show resolved Hide resolved
@@ -87,10 +87,10 @@ def save_input_values(inputs: Sequence[np.ndarray],


def _setup_mlir_crash_reproducer(
function: Callable[[Any], Any],
function: Any, # pytype doesn't support arbitrary Callable[*args, **kwargs]
Copy link
Contributor

Choose a reason for hiding this comment

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

I found a thread about this somewhere. I think it said there is a way to write this. Maybe worth investigating, but can be a followup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SGTM. Could you send a link?

Copy link
Contributor

Choose a reason for hiding this comment

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

python/mypy#5876. I would have to understand pytype better to understand all that's going on in that thread. python/typing#264 (comment) and https://stackoverflow.com/q/57837609 may also help

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, I was looking through those. The first one would be ideal if it were implemented. But the other two make assumptions about types that the function can ingest, which would be inappropriate in our case.

@phoenix-meadowlark phoenix-meadowlark merged commit cd5c990 into iree-org:main Oct 22, 2020
@rsuderman rsuderman mentioned this pull request Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Relating to build systems, CI, or testing integrations Relating to high-level frontend integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants