-
Notifications
You must be signed in to change notification settings - Fork 712
Arm backend: Enable mypy for tests #15672
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15672
Note: Links to docs will display an error until the docs builds have been completed. ⏳ 1 Pending, 6 Unrelated FailuresAs of commit c501ea7 with merge base 747fc6f ( BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
0a4b843 to
ec1f6a5
Compare
|
I added all needes commits to this PR to make life easier. |
|
@mergennachin and/or @SS-JIA there is one line in this PR that is not 100% Arm specific, it adds the "new" src folder to .mypy.ini this clean up some lintrunner problem as project starting to use it also. e.g. I'm not sure if this is something you would like to test internally or not, we let this PR be up for some days so you get a chance to weight in. As there is a inflow of PR that might breaks this as it not enabled until this lands, we will need to rebase and maybe add more lintrunner fixes in this PR before merging and we hope/plan to do this "Europe" time so can test it and retrigger some random PR to make sure there is no problem and do a fast fix or revert while less people is active in the project. |
|
Fix incoming for the actual lint error. :) |
And the lintrunner-mypy got green with that commit. So Im confident it is running the right thing. |
SS-JIA
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ok, MYPY/lintrunner is not run internally
|
Thanks |
|
Ill rebase and check that lintrunner is still happy :) |
Fix mypy warning about type. Signed-off-by: per.held@arm.com Change-Id: I09a5f75943c12b304a2e4d4ff6af8739021aeb44
Some missing imports for third-party libraries used in stable_diffusion had ignores added to them. Signed-off-by: per.held@arm.com Change-Id: Icd86edd39ae4541813bc84c07fdab90571da98c2
Add src to mypy_path to get rid of lots of missing import warnings. Signed-off-by: per.held@arm.com Change-Id: I4ed1a08cfe185aa956c6408fefa5aadeb567d4c1
Add test directory in arm backend to lintrunner. Except the ops directory which has to be fixed later. About "disallow_untyped_decorators = False": The only way to make mypy happy seems to be a wrapper around the decorator for each file it is used. I have read numerous of threads about it and there seems to be no good solution. Hence the added ignore for backends/arm. Signed-off-by: per.held@arm.com Change-Id: I0430041cb3308b51798d74da61cc4d310966772a
Add ignore for import as well, since on github these packages are not installed and thus its a import-not-found error and not untyped as it becomes locally if you have the packages installed. Signed-off-by: per.held@arm.com Change-Id: Iba7b579f8f95991aca26054698e1d357dc14a853
Signed-off-by: per.held@arm.com Change-Id: I351b955621c8f5b807e4ead2722d3b7f0d98df51
b1bdf8e to
c501ea7
Compare
|
Failed are unrelated. Ive rebased locally and it still passes lintrunner. |
Add test directory in arm backend to lintrunner. Except the ops directory which has to be fixed later.
About "disallow_untyped_decorators = False":
The only way to make mypy happy seems to be a wrapper around the decorator for each file it is used. I have read numerous of threads about it and there seems to be no good solution. Hence the added ignore for backends/arm.
Signed-off-by: per.held@arm.com
Change-Id: I0430041cb3308b51798d74da61cc4d310966772a
cc @freddan80 @per @zingo @oscarandersson8218 @digantdesai