Conversation
|
@Kludex This is an amazing pre-commit hook which has helped reduce the typing issues. I have made some improvements which makes it efficient by handling exceptions . Please take a look and review . Thank you! |
Kludex
left a comment
Kludex
left a comment
There was a problem hiding this comment.
I'm happy to accept any PR that is self-contained.
| print( | ||
| "promypy failed with exit status code 2. Please raise this issue with the developer." | ||
| ) | ||
| print("Error details: ", api_result) |
There was a problem hiding this comment.
Was added as log to debug incase there is a timeout to reach out to whoever is maintaining the pre-commit hook.
There was a problem hiding this comment.
Can we open a different PR for this?
| directory: str = ".", | ||
| mypy_args: Optional[str] = None, | ||
| timeout: int = 10, | ||
| timeout: int = 30, |
There was a problem hiding this comment.
mypy was too slow to process even the simplest nested files. Since there were huge number of files which were resulting in timeout exception, we had to change the default instead of passing as an arg.
There was a problem hiding this comment.
Is this that slow on the latest mypy releases?
But you can pass a different timeout for your use case.
| if filename not in files_to_ignore: | ||
| output.append(line) | ||
| except TimeoutError: # pragma: no cover | ||
| progress.console.print("TimeoutError") |
There was a problem hiding this comment.
Can we speed up the tool instead?
There was a problem hiding this comment.
That could be something done if they are going to continue using promypy for type checking purposes. This PR is just to have these changes on master in order to avoid using the forked repo.
There was a problem hiding this comment.
This seems a bit selfish. The license is MIT, you are free to fork, and publish under a different PyPI domain, as long as you don't remove the license.
|
Is Deliverect using this? |
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
Yes, currently they are. |
|
The tests are not passing. PR welcome to drop Python 3.7, or maybe you can just fix the problem on the pipeline? I'm not willing to accept this PR as is. I'd prefer to have minimal PRs. This PR introduces opinionated behavior changes without previous discussion. That said, I'm happy to continue reviewing PRs if you are willing to open small PRs, with tests. |
2 participants
Description
This PR intends to rectify the pool process through which files are being checked and dumped for type checking using mypy api.
Changes
Promypy dump command :
Exception files :
Previously implemented pool process was not handling timeout errors returned by mypy api. The issue for timeout was because of deeply nested objects . Hence we had to classify those files as exception files so it doesn't fail silently. This is one of the important scenarios to be handled as when verifying JSON payloads, this highlights the exception files which failed type checking.
Processed files :
Introduce set of processed files as we filter out files that failed due to an exception in promypy api. Modify the way we loop in the list of files with that of
as_completedfutures. Previously there was a mismatch in the error that is being matched with that of filename.Promypy check command :
Introduce the same pool process to check the files which are passed to mypy api.
Both dump and check have been improvised to handle exceptions and fix the mismatch when processing the result of the pool.
There are also some version which are bumped in the process of updating libraries used .