KEMBAR78
Enable mypy lintrunner, Part 4 (util/*) by mergennachin · Pull Request #7496 · pytorch/executorch · GitHub
Skip to content

Conversation

@mergennachin
Copy link
Contributor

@mergennachin mergennachin commented Jan 3, 2025

Fixes #7441

REVIEW THIS COMMIT ONLY : Enable mypy lintrunner, Part 4 (util/*) 00d60e4

Ignore the first two commits, they are part of separate PRs.

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 3, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7496

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit bc81349 with merge base 7a2dc47 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 3, 2025
@mergennachin mergennachin changed the title lints part 4 Enable mypy lintrunner, Part 4 (util/*) Jan 3, 2025
ignore_missing_imports = True

[mypy-buck_util]
[mypy-docutils.*]
Copy link
Contributor

Choose a reason for hiding this comment

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

why these one has ignore missing imports=true? wasnt before? Same for pandas

Copy link
Contributor

Choose a reason for hiding this comment

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

I am guessing to make linter pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i just sorted alphabetically

and some third party libraries, it does the best effort to infer the types, like, follow_untyped_imports=True

if it can't traverse due to missing symbols, linters complains, so i'm just ignoring these imports altogether. ignore_missing_imports

final_argument_whitespace = True

def run(self) -> List[nodes.Node]:
def run(self, params, alt, _) -> List[nodes.Node]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is consistent with the base class signature. run is overriden method.

Copy link
Contributor

@kimishpatel kimishpatel left a comment

Choose a reason for hiding this comment

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

looks good to me

@facebook-github-bot
Copy link
Contributor

@mergennachin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mergennachin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mergennachin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@mergennachin mergennachin merged commit 3fdff26 into main Jan 6, 2025
45 checks passed
@mergennachin mergennachin deleted the lints_part_4 branch January 6, 2025 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. topic: not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable mypy typechecker tracker

4 participants