Skip to content

Remove obsolete "E501" line in pyproject.toml #5539

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

Merged
merged 2 commits into from
Apr 11, 2025

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Feb 21, 2025

Description

With PR #4912 we removed black:

-# Black, the code formatter, natively supports pre-commit
-- repo: https://github.com/psf/black-pre-commit-mirror
-  rev: "23.10.1" # Keep in sync with blacken-docs
-  hooks:
-  - id: black

So this isn't true anymore:

"E501", # Line too long (Black is enough)

I tried to remove that line, but it doesn't make a difference, ruff doesn't produce the E501 error. See dummy test case, which doesn't trigger any ruff errors.

It would be nice to see that error, even if we need to fix manually.

@henryiii: Do you happen to know what suppresses the E501?

Suggested changelog entry:

@timohl
Copy link
Contributor

timohl commented Feb 21, 2025

As far as I can see, Ruff does not include E501 by default, so it has to be added to extend-select.
See docs.astral.sh/ruff/configuration for the default settings:
Only "E4", "E7", "E9" are in select but not E5.

@henryiii
Copy link
Collaborator

Yes, this got missed when we moved from select to extend-select.

@rwgk
Copy link
Collaborator Author

rwgk commented Feb 26, 2025

Thanks for the feedback! — I'll get back to this after #5542 is merged.

@rwgk rwgk force-pushed the remove_ruff_ignore_E501 branch from 7b209be to 4676ef9 Compare April 9, 2025 03:42
@rwgk
Copy link
Collaborator Author

rwgk commented Apr 9, 2025

@henryiii We're getting 249 E501 Line too long errors after adding "E5" to extend-select.

This is with the default line-length = 88

Here are all the line lengths > 88:

89 89 89 89 89 89 89 89 89 89 90 90 90 90 90 90 90 90 90 90
90 90 90 90 90 90 91 91 91 91 91 91 91 91 91 91 91 91 91 91
91 91 91 92 92 92 92 92 92 92 92 92 92 92 92 93 93 93 93 93
93 93 93 93 93 94 94 94 94 94 94 94 94 94 94 94 95 95 95 95
95 95 95 95 95 95 95 95 95 95 95 96 96 96 96 96 96 96 96 96
96 96 96 96 96 96 96 97 97 97 97 97 97 97 97 97 97 97 97 97
97 97 97 97 98 98 98 98 98 98 98 98 98 99 99 99 99 99 99 99
99 99 99 99 99 99 99 99 100 100 100 100 100 100 100 100 100 101 101 101
101 101 102 102 102 102 102 102 103 103 103 103 103 103 103 104 104 104 104 104
105 106 106 107 107 107 107 107 107 107 107 108 108 108 109 109 110 111 111 111
113 113 113 114 114 114 114 115 115 115 115 116 116 116 116 116 117 118 119 119
120 121 124 124 124 125 127 129 130 130 131 133 134 140 140 140 142 145 145 146
153 158 158 158 163 163 189 220 221

Where do we want to cut off?

@rwgk rwgk changed the title [WIP] Remove ruff ignore E501 [WIP] Add "E5" to [tool.ruff.lint] extend-select to enable E501 Line too long errors Apr 9, 2025
@rwgk rwgk mentioned this pull request Apr 9, 2025
@henryiii
Copy link
Collaborator

Ruff format makes these pretty rare, the only times it hits are for things like long strings. I'd say I'd be fine to leave the default (don't enable E5) since we use ruff format. There's a reason it's on in the default selection. :)

pyproject.toml Outdated
@@ -55,6 +55,7 @@ extend-select = [
"N", # pep8-naming
"ARG", # flake8-unused-arguments
"C4", # flake8-comprehensions
"E5", # pycodestyle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And drop this one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

I'd prefer a limit (same as for code), but oh well.

@rwgk
Copy link
Collaborator Author

rwgk commented Apr 10, 2025

Ruff format makes these pretty rare, the only times it hits are for things like long strings. I'd say I'd be fine to leave the default (don't enable E5) since we use ruff format. There's a reason it's on in the default selection. :)

Is my understanding correct: Do you want to leave the line-length for long strings unlimited?

@rwgk rwgk force-pushed the remove_ruff_ignore_E501 branch from 4676ef9 to eb85997 Compare April 11, 2025 03:45
@rwgk rwgk changed the title [WIP] Add "E5" to [tool.ruff.lint] extend-select to enable E501 Line too long errors Remove obsolete "E501" line in pyproject.toml Apr 11, 2025
@rwgk rwgk marked this pull request as ready for review April 11, 2025 03:47
@rwgk rwgk merged commit f3c1913 into pybind:master Apr 11, 2025
2 checks passed
@rwgk rwgk deleted the remove_ruff_ignore_E501 branch April 11, 2025 06:09
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Apr 11, 2025
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Apr 11, 2025
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.

3 participants