[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.
| 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.
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.