You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/06/07 12:49:04 UTC
[GitHub] [tvm] gigiblender opened a new pull request, #11600: Enable pylint for the python tests
gigiblender opened a new pull request, #11600:
URL: https://github.com/apache/tvm/pull/11600
This PR enables pylint for the tests and addresses #11414.
There is a large number of tests that have formatting errors. See the file attached.
[out.txt](https://github.com/apache/tvm/files/8853259/out.txt)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on pull request #11600: Enable pylint for the python tests
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #11600:
URL: https://github.com/apache/tvm/pull/11600#issuecomment-1148891756
@gigiblender is there anyway to make it error for new tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gigiblender commented on pull request #11600: Enable pylint for the python tests
Posted by GitBox <gi...@apache.org>.
gigiblender commented on PR #11600:
URL: https://github.com/apache/tvm/pull/11600#issuecomment-1149797600
@Mousius I don't see a clean way to do this.
We could have a temporary `pylintrc` file, used only for the tests, that passes the list in #11414 as the [ignore parameter.](https://github.com/apache/tvm/blob/6dc0c624cdd8fb9d7fdd2194a755b0dffbe2de93/tests/lint/pylintrc#L29). However, new tests in existing files would also be ignored in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gigiblender commented on pull request #11600: Enable pylint for the python tests
Posted by GitBox <gi...@apache.org>.
gigiblender commented on PR #11600:
URL: https://github.com/apache/tvm/pull/11600#issuecomment-1156297735
>
@Mousius, I think the second option sounds better. We can add subdirectories to the list as we fix them and collapse the list in a single entry once all the tests pass the linter. I think I can close this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gigiblender closed pull request #11600: Enable pylint for the python tests
Posted by GitBox <gi...@apache.org>.
gigiblender closed pull request #11600: Enable pylint for the python tests
URL: https://github.com/apache/tvm/pull/11600
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on pull request #11600: Enable pylint for the python tests
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #11600:
URL: https://github.com/apache/tvm/pull/11600#issuecomment-1156208860
Argh @gigiblender, I didn't realise we would only be able to specify `test_pooling.py`, that means `test_cmsisnn` is ignored by mistake in the last test block - ideally we'd disable all of them and then enable them as we merge PRs to fix them, unsure if we can do that or if we just have to keep adding directories to the list and clean up later 🤔 any ideas @gigiblender ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on pull request #11600: Enable pylint for the python tests
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #11600:
URL: https://github.com/apache/tvm/pull/11600#issuecomment-1156311759
Sounds good to me @gigiblender, thanks for raising this and kickstarting the great linting of 2022!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on pull request #11600: Enable pylint for the python tests
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #11600:
URL: https://github.com/apache/tvm/pull/11600#issuecomment-1151670261
> @Mousius I don't see a clean way to do this.
>
> We could have a temporary `pylintrc` file, used only for the tests, that passes the list in #11414 as the [ignore parameter.](https://github.com/apache/tvm/blob/6dc0c624cdd8fb9d7fdd2194a755b0dffbe2de93/tests/lint/pylintrc#L29). However, new tests in existing files would also be ignored in this case.
Hmmm, I'm unsure whether we want to fill the logs with pylint without actually halting the build? Methinks it's worth doing as you suggested and filtering out the files we know will fail - we then have to lint them ourselves as quickly as we can 😿
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gigiblender commented on pull request #11600: Enable pylint for the python tests
Posted by GitBox <gi...@apache.org>.
gigiblender commented on PR #11600:
URL: https://github.com/apache/tvm/pull/11600#issuecomment-1152598646
> > @Mousius I don't see a clean way to do this.
> > We could have a temporary `pylintrc` file, used only for the tests, that passes the list in #11414 as the [ignore parameter.](https://github.com/apache/tvm/blob/6dc0c624cdd8fb9d7fdd2194a755b0dffbe2de93/tests/lint/pylintrc#L29). However, new tests in existing files would also be ignored in this case.
>
> Hmmm, I'm unsure whether we want to fill the logs with pylint without actually halting the build? Methinks it's worth doing as you suggested and filtering out the files we know will fail - we then have to lint them ourselves as quickly as we can crying_cat_face
@Mousius, I applied your suggestion. Please have a look when you get the time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org