misc: reduce max-line-length ruff setting to 130 [NFC] #3869
Conversation
superlopuh
requested review from
compor,
alexarice,
math-fehr,
qaco,
jorendumoulin and
webmiche
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| with pytest.raises( | ||
| VerifyException, | ||
| match=f"The following constraints were not satisfied:\n{IntData(7)} should " | ||
| f"hold a value less than 5\n{IntData(7)} should hold a value greater than 8", | ||
| ): |
There was a problem hiding this comment.
the only functional-ish change
| # (2) There is no ptr.to_ptr operation currently proposed, but there is a memref.to_ptr. We do this so that we can feed the results of convert-memref-to-ptr pass without any conflict. | ||
| ######## | ||
|
|
||
| """ |
There was a problem hiding this comment.
this was a weird pseudo doc comment I made it an actual doc comment
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3869 +/- ##
=======================================
Coverage 91.24% 91.24%
=======================================
Files 463 464 +1
Lines 57758 57780 +22
Branches 5572 5573 +1
=======================================
+ Hits 52701 52723 +22
Misses 3630 3630
Partials 1427 1427 ☔ View full report in Codecov by Sentry. |
tests/dialects/test_transform.py
Outdated
| """%0, %1, %2 = "transform.structured.tile_using_for"(%3) <{static_sizes = array<i32: 8, 8>}> : (!transform.any_value) -> (!transform.any_op, !transform.any_op, !transform.any_op)""", # noqa: E501 | ||
| None, | ||
| ) |
There was a problem hiding this comment.
Do we need these noqa comments here since we ignore all test files above ("tests/*" = ["E501"])?
When I remove it, ruff check doesn't trigger for me unless I also remove the aforementioned line from pyproject.toml.
There was a problem hiding this comment.
Ah you're right I'll do a pass
|
I'm happy with 100 chars of line length, although most OSS Python project seem to adhere to the PEP8 88 gospel. Related also to #1274 (maybe close it with a comment and pointing to this PR?) |
|
Let's keep that open for now, and close when/if we get to 100? |
I don't know if we'll ever get to 88, but it feels like it would be good to get the line lengths a little more under control. In this PR, I update the line length, ignore all test files, ignore the line length limit on all assembly format strings, reflow some doc strings, and split up some inline strings in the code.
After this change, there are 840 errors when I set the line length to 88, and 313 if I set it to 100. I think without some sort of AI "fix everything" button it's a waste of time to go shorter, but our current setting of 300 definitely seems to big.
This PR contains about 60 whitespace changes.