[TVMScript] Optionally use ruff format instead of black
#16876
Conversation
The `ruff format` tool is significantly faster than the `black` formatter. For some particularly long TVMScript modules, using it can reduce the time required to show a formatted module from ~5 minutes to ~1 minute. This commit updates the `.show()` function to apply the optionally formatting using `ruff format` if available, falling back to `black` otherwise.
quic-sanirudh
left a comment
quic-sanirudh
left a comment
There was a problem hiding this comment.
This a great addition, thanks a lot. Just had one minor doubt about whether constraining to specific version of ruff is needed, but otherwise this will be really useful.
| def formatter(code_str): | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("once", UserWarning) | ||
| ruff_install_cmd = sys.executable + " -m pip install ruff" |
There was a problem hiding this comment.
Should we restrict to a specific version of ruff similar to a specific black version we've specified below?
There was a problem hiding this comment.
I'm uncertain, but would lean instead toward removing the specific version of black, if we want consistency. Specifying a specific version of black is useful, since then it can be matched to the CI's version, in case a developer also uses black either for format-on-save or as a pre-commit hook. However, the version number in the warning has become out of sync with the CI's version (warning message was added in November 2022, but the CI was bumped from 22.3.0 to 22.12.0 in March 2023).
Since there isn't really a good way to keep them in sync (nor any ruff version in the CI with which to be in sync), I'd lean toward removing the versions from the warning message altogether.
There was a problem hiding this comment.
Right yeah, that makes more sense. It is very hard to keep them in sync. Perhaps after we remove the version info from the warning message, we can just add a line saying something like "For version compatibility with CI, please check docker/Dockerfile.ci_lint" if the version info has to be specified.
Thanks for the response.
2 participants
The
ruff formattool is significantly faster than theblackformatter. For some particularly long TVMScript modules, using it can reduce the time required to show a formatted module from ~5 minutes to ~1 minute. This commit updates the.show()function to apply the optionally formatting usingruff formatif available, falling back toblackotherwise.