Add cython-lint to pre-commit #57699
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarcoGorelli
force-pushed
the
cython-lint
branch
from
c32f0df to
68435ab
Compare
edoakes
reviewed
Oct 14, 2025
Collaborator
edoakes
left a comment
edoakes
left a comment
There was a problem hiding this comment.
Looks like a nice addition. Are the changes here all of the errors from enabling the linter?
python/ray/_raylet.pyx
Outdated
|
|
||
| def function_executor(*arguments, **kwarguments): | ||
| function = execution_info.function | ||
| function = execution_info.function # no-cython-lint |
Collaborator
There was a problem hiding this comment.
what needed to be skipped here?
Contributor
Author
There was a problem hiding this comment.
Because function shadows the globally-imported symbol here:
Line 60 in 68435ab
We can either turn off the lint of this line, or rename this local function variable. Do you have a preference?
Collaborator
There was a problem hiding this comment.
let's rename the local variable to avoid any # no-cython-lint
python/ray/_raylet.pyx
Outdated
| increase_recursion_limit() | ||
|
|
||
| eventloop, async_thread = self.get_event_loop( | ||
| eventloop, _async_thread = self.get_event_loop( |
Collaborator
There was a problem hiding this comment.
to follow convention in the rest of the code, let's just do:
Suggested change
| eventloop, _async_thread = self.get_event_loop( | |
| eventloop, _ = self.get_event_loop( |
| rev: v0.17.0 | ||
| hooks: | ||
| - id: cython-lint | ||
| args: [--no-pycodestyle] |
Contributor
Author
There was a problem hiding this comment.
pycodestyle runs some more nitpicky lints, like
/home/marcogorelli/ray-dev/python/ray/_raylet.pyx:2324:89: E501 line too long (108 > 88 characters)
/home/marcogorelli/ray-dev/python/ray/_raylet.pyx:2330:5: E303 too many blank lines (2)
/home/marcogorelli/ray-dev/python/ray/_raylet.pyx:2528:6: E111 indentation is not a multiple of 4
/home/marcogorelli/ray-dev/python/ray/_raylet.pyx:2528:6: E117 over-indented
so for a first iteration I kept them out
Collaborator
There was a problem hiding this comment.
these look good to me too, maybe open a 2nd PR that removes the --no-pycodestyle?
ray-gardener
bot
added
core
Contributor
Author
|
thanks for your review! yes, that's right |
MarcoGorelli
force-pushed
the
cython-lint
branch
2 times, most recently
from
debd15b to
689d10d
Compare
Collaborator
|
Triggered CI run and enabled auto-merge. If CI passes the PR will merge. If not, please ping me. Thanks again for the contribution! |
Contributor
Author
|
thanks @edoakes !
looks like it didn't go, should i fetch upstream and rebase? |
auto-merge was automatically disabled
October 22, 2025 13:08
Head branch was pushed to by a user without write access
MarcoGorelli
force-pushed
the
cython-lint
branch
from
689d10d to
a2d8fd2
Compare
Signed-off-by: Marco Gorelli <[email protected]>
MarcoGorelli
force-pushed
the
cython-lint
branch
from
a2d8fd2 to
bf2a903
Compare
edoakes
approved these changes
Oct 31, 2025
YoussefEssDS
pushed a commit
to YoussefEssDS/ray
that referenced
this pull request
Nov 8, 2025
I was testing out `cython-lint` and figured I'd open a PR to see if there was interest This flags some issues here such as: - unused imports - unused variables - "pointless string statements" - f-string without placeholders Signed-off-by: Marco Gorelli <[email protected]>
landscapepainter
pushed a commit
to landscapepainter/ray
that referenced
this pull request
Nov 17, 2025
I was testing out `cython-lint` and figured I'd open a PR to see if there was interest This flags some issues here such as: - unused imports - unused variables - "pointless string statements" - f-string without placeholders Signed-off-by: Marco Gorelli <[email protected]>
Aydin-ab
pushed a commit
to Aydin-ab/ray-aydin
that referenced
this pull request
Nov 19, 2025
I was testing out `cython-lint` and figured I'd open a PR to see if there was interest This flags some issues here such as: - unused imports - unused variables - "pointless string statements" - f-string without placeholders Signed-off-by: Marco Gorelli <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>
SheldonTsen
pushed a commit
to SheldonTsen/ray
that referenced
this pull request
Dec 1, 2025
I was testing out `cython-lint` and figured I'd open a PR to see if there was interest This flags some issues here such as: - unused imports - unused variables - "pointless string statements" - f-string without placeholders Signed-off-by: Marco Gorelli <[email protected]>
Future-Outlier
pushed a commit
to Future-Outlier/ray
that referenced
this pull request
Dec 7, 2025
I was testing out `cython-lint` and figured I'd open a PR to see if there was interest This flags some issues here such as: - unused imports - unused variables - "pointless string statements" - f-string without placeholders Signed-off-by: Marco Gorelli <[email protected]> Signed-off-by: Future-Outlier <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was testing out
cython-lintand figured I'd open a PR to see if there was interestThis flags some issues here such as:
Why are these changes needed?
Related issue number
Checks
git commit -s) in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.