Skip to content

fix(lint): ignore ruff PLR0917 so trunk passes ruff check - #178

Closed
claudespice wants to merge 1 commit into
spiceai:trunkfrom
claudespice:fix/ruff-plr0917-positional-args
Closed

fix(lint): ignore ruff PLR0917 so trunk passes ruff check#178
claudespice wants to merge 1 commit into
spiceai:trunkfrom
claudespice:fix/ruff-plr0917-positional-args

Conversation

@claudespice

Copy link
Copy Markdown
Contributor

ruff check spicepy tests — the command the Lint with ruff job runs — fails on trunk itself with two errors, so every open pull request inherits a red ruff job regardless of what it changes:

PLR0917 Too many positional arguments (6 > 5)
   --> spicepy/_client.py:239:9
PLR0917 Too many positional arguments (7 > 5)
   --> spicepy/_client.py:332:9

Verified against a clean checkout of trunk (a199cfa) with ruff 0.16.1: two errors, no branch involved.

Why ignore rather than refactor

The lint config already ignores PLR0913 ("Too many arguments in function definition"), and both flagged constructors carry an in-code # pylint: disable=R0917 — pylint's name for this same rule. The decision to allow these signatures was already made and recorded in two places; only ruff's half of the pair was missing from ignore.

Client.__init__ is the public SDK constructor. Satisfying the rule for real would mean making its parameters keyword-only, which breaks every caller that passes them positionally — not an acceptable trade for a lint warning the project has already opted out of under its pylint name.

Test plan

  • ruff check spicepy testsAll checks passed! (was: Found 2 errors.)
  • black --check spicepy tests → unchanged, 22 files clean
  • No source files touched; the change is two lines of lint configuration.

`ruff check spicepy tests` fails on trunk with two PLR0917
"Too many positional arguments" errors, both in spicepy/_client.py.
Every open pull request inherits the failure, so the ruff lint job is
red regardless of what the branch changes.

The lint config already ignores PLR0913 ("Too many arguments"), and
both flagged constructors carry an in-code `# pylint: disable=R0917` --
the pylint name for the same rule -- so the intent to allow them was
already recorded; only ruff's half of the pair was missing.

Client.__init__ is the public SDK constructor, so making its
parameters keyword-only to satisfy the rule would break callers that
pass them positionally.
@claudespice

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate — #176 already carries this exact fix, and it predates this PR by five days.

I opened this after confirming ruff check fails on a clean trunk, but I did not check the open PRs from other authors first. #176 adds PLR0917 to the same ignore list with the same reasoning (positional counterpart of the already-ignored PLR0913; making the client constructor parameters keyword-only would be a breaking API change), and additionally fixes the spice add spiceai/quickstart registry failure in test.yml that this PR does not touch. #176 is strictly the better change and its checks are green.

Sorry for the noise.

@claudespice claudespice closed this Aug 1, 2026
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.

1 participant