Skip to content
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

Reformat Python code with yapf. #2589

Merged
merged 4 commits into from
Nov 3, 2023
Merged

Reformat Python code with yapf. #2589

merged 4 commits into from
Nov 3, 2023

Conversation

jlebar
Copy link
Collaborator

@jlebar jlebar commented Nov 3, 2023

I've add an option to yapf to do what we want for long lines, see
google/yapf#1177. We can now have a real Python
formatter, yay!

To make this PR, I ran my modified yapf over the repository, then looked
over the full diff. Where yapf was mangling the param list of long
function decls/calls (mostly kernels), I manually added # to put
linebreaks where we want. I fixed up other formatting too -- mostly
adding or removing a trailing comma from lists.

Overall, trailing # was sufficient to get formatting similar to our
current code. I didn't have to disable yapf anywhere.

@jlebar jlebar requested a review from ptillet as a code owner November 3, 2023 01:13
@ptillet
Copy link
Collaborator

ptillet commented Nov 3, 2023

LGTM. I like this better than black :)

I've add an option to yapf to do what we want for long lines, see
google/yapf#1177.  We can now have a real Python
formatter, yay!

To make this PR, I ran my modified yapf over the repository, then looked
over the full diff.  Where yapf was mangling the param list of long
function decls/calls (mostly kernels), I manually added `#` to put
linebreaks where we want.  I fixed up other formatting too -- mostly
adding or removing a trailing comma from lists.

Overall, trailing `#` was sufficient to get formatting similar to our
current code.  I didn't have to disable yapf anywhere.
@jlebar
Copy link
Collaborator Author

jlebar commented Nov 3, 2023

Thanks a lot, Phil. Turns out you can have it in any color you want. :)

ptillet and others added 3 commits November 2, 2023 18:58
We don't want to auto-format this file, because the test depends on the
exact formatting of the code in the file.
Without this, the line-info test fails.

  python -m pytest python/triton/compiler/code_generator.py

Fails 10/10 times without this blank line, passes 10/10 times with the
blank line.

The failure is in call_noinline and multi_files, both of which generate
incorrect line numbers.

I have absolutely no idea what is going on.
@ptillet ptillet merged commit df08301 into main Nov 3, 2023
4 checks passed
@ptillet ptillet deleted the yapf branch November 3, 2023 03:44
pingzhuu pushed a commit to siliconflow/triton that referenced this pull request Apr 2, 2024
I've add an option to yapf to do what we want for long lines, see
google/yapf#1177.  We can now have a real Python
formatter, yay!

To make this PR, I ran my modified yapf over the repository, then looked
over the full diff.  Where yapf was mangling the param list of long
function decls/calls (mostly kernels), I manually added `#` to put
linebreaks where we want.  I fixed up other formatting too -- mostly
adding or removing a trailing comma from lists.

Overall, trailing `#` was sufficient to get formatting similar to our
current code.  I didn't have to disable yapf anywhere.

---------

Co-authored-by: Phil Tillet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants