Skip to content

fix: update ruff to 0.16 and address its lint findings - #305

Merged
nielspardon merged 2 commits into
substrait-io:mainfrom
nielspardon:style/ruff-0.16-lint
Oct 13, 2026
Merged

nielspardon merged 2 commits into
substrait-io:mainfrom
nielspardon:style/ruff-0.16-lint

Conversation

@nielspardon

@nielspardon nielspardon commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

ruff 0.16 turns on a much larger default rule set, so bumping the pixi pin
needs these fixes alongside it.

Two of the findings were real bugs:

  • substrait.narwhals.__all__ listed the classes instead of their names,
    so from substrait.narwhals import * raised a TypeError.
  • test_wildcard_match passed the builtin bool() (i.e. False) instead of
    the boolean() type builder, so it never exercised a boolean argument.

TRY002/TRY004 are ignored rather than fixed: the public API raises plain
Exception, and moving to typed exceptions is a separate, user-visible
change. DTZ001 is ignored in tests, where naive datetimes model Substrait's
timezone-less timestamps.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: substrait-io/substrait-python/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9ced1064-89ee-4758-83ad-95a0afc1b306
📥 Commits

Reviewing files that changed from the base of the PR and between 8d3c242 and bbae5d3.

⛔ Files ignored due to path filters (1)
  • pixi.lock is excluded by !**/*.lock
📒 Files selected for processing (34)
  • .pre-commit-config.yaml
  • examples/adbc_example.py
  • pyproject.toml
  • src/substrait/builders/extended_expression.py
  • src/substrait/builders/plan.py
  • src/substrait/builders/type.py
  • src/substrait/dataframe/__init__.py
  • src/substrait/dataframe/dtypes.py
  • src/substrait/dataframe/expr.py
  • src/substrait/dataframe/extension_relations.py
  • src/substrait/dataframe/frame.py
  • src/substrait/derivation_expression.py
  • src/substrait/extension_registry/__init__.py
  • src/substrait/extension_registry/collector.py
  • src/substrait/extension_registry/exceptions.py
  • src/substrait/extension_registry/function_entry.py
  • src/substrait/extension_registry/registry.py
  • src/substrait/extension_registry/signature_checker_helpers.py
  • src/substrait/narwhals/__init__.py
  • src/substrait/narwhals/dataframe.py
  • src/substrait/proto/__init__.py
  • src/substrait/simple_extension_utils.py
  • src/substrait/sql/sql_to_substrait.py
  • src/substrait/type_inference.py
  • src/substrait/utils/__init__.py
  • src/substrait/utils/display.py
  • tests/builders/plan/test_lateral_join.py
  • tests/builders/plan/test_schema_memo.py
  • tests/dataframe/test_dtypes.py
  • tests/dataframe/test_functions.py
  • tests/dataframe/test_literals.py
  • tests/extension_registry/test_collector.py
  • tests/extension_registry/test_registry_lookup.py
  • tests/test_type_inference.py

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: substrait-io/substrait-python/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 077f365f-4294-4e31-830f-cae6e5cac472
📥 Commits

Reviewing files that changed from the base of the PR and between 6dc4111 and 8d3c242.

⛔ Files ignored due to path filters (1)
  • pixi.lock is excluded by !**/*.lock
📒 Files selected for processing (34)
  • .pre-commit-config.yaml
  • examples/adbc_example.py
  • pyproject.toml
  • src/substrait/builders/extended_expression.py
  • src/substrait/builders/plan.py
  • src/substrait/builders/type.py
  • src/substrait/dataframe/__init__.py
  • src/substrait/dataframe/dtypes.py
  • src/substrait/dataframe/expr.py
  • src/substrait/dataframe/extension_relations.py
  • src/substrait/dataframe/frame.py
  • src/substrait/derivation_expression.py
  • src/substrait/extension_registry/__init__.py
  • src/substrait/extension_registry/collector.py
  • src/substrait/extension_registry/exceptions.py
  • src/substrait/extension_registry/function_entry.py
  • src/substrait/extension_registry/registry.py
  • src/substrait/extension_registry/signature_checker_helpers.py
  • src/substrait/narwhals/__init__.py
  • src/substrait/narwhals/dataframe.py
  • src/substrait/proto/__init__.py
  • src/substrait/simple_extension_utils.py
  • src/substrait/sql/sql_to_substrait.py
  • src/substrait/type_inference.py
  • src/substrait/utils/__init__.py
  • src/substrait/utils/display.py
  • tests/builders/plan/test_lateral_join.py
  • tests/builders/plan/test_schema_memo.py
  • tests/dataframe/test_dtypes.py
  • tests/dataframe/test_functions.py
  • tests/dataframe/test_literals.py
  • tests/extension_registry/test_collector.py
  • tests/extension_registry/test_registry_lookup.py
  • tests/test_type_inference.py
💤 Files with no reviewable changes (2)
  • src/substrait/proto/init.py
  • src/substrait/extension_registry/exceptions.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Refactor
    • Updated type annotations and simplified several internal operations without changing documented behavior.
  • Chores
    • Updated code-formatting and linting tools and their project configuration.
  • Tests
    • Refined test coverage and corrected test inputs and assertions.
  • No user-facing features or behavior changes are included in this release.

The pull request upgrades Ruff and updates its configuration. It replaces legacy typing forms with modern union syntax across builder, DataFrame, and supporting APIs. It also makes small changes to SQL translation, expression evaluation, examples, and tests.

Changes: Python tooling and code modernization

Layer / File(s) Summary
Ruff upgrade and lint cleanup
.pre-commit-config.yaml, pyproject.toml, examples/adbc_example.py, src/substrait/dataframe/*, src/substrait/extension_registry/*, src/substrait/narwhals/__init__.py, src/substrait/proto/__init__.py, src/substrait/utils/display.py
The Ruff revision and dependency constraint are updated, and lint settings change. Related edits adjust context managers, export declarations, slot ordering, exception-class bodies, and lint annotations.
Builder and expression annotations
src/substrait/builders/*
Builder and expression annotations use `
DataFrame and Narwhals annotations
src/substrait/dataframe/*, src/substrait/narwhals/dataframe.py
DataFrame and Narwhals annotations use direct forward references and `
Registry, translation, and validation updates
src/substrait/derivation_expression.py, src/substrait/extension_registry/*, src/substrait/simple_extension_utils.py, src/substrait/sql/sql_to_substrait.py, src/substrait/type_inference.py, src/substrait/utils/__init__.py, tests/*
Supporting annotations and selected expressions are updated. SQL AST keys use iterator access; derivation expressions use combined parameter handling and list comprehensions. Tests update selected error-message checks, imports, context managers, and inputs.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 8d3c2

No actionable regression is established in the reviewed changes; the PR is ready for normal repository checks.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 55.74% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 235 functions across 30 files. (2 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title is a concise Conventional Commit message and accurately summarizes the Ruff update and lint fixes.
Description check Passed The description provides the rationale, identifies the main fixes, and explains the intentional lint-rule exceptions. No required information is missing for this non-breaking change.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 55.74% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 235 functions across 30 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nielspardon nielspardon changed the title fix: address lint findings from ruff 0.16 fix: update ruff to 0.16 and address its lint findings Oct 13, 2026
ruff 0.16 turns on a much larger default rule set, so bumping the pixi pin
needs these fixes alongside it.

Two of the findings were real bugs:

- `substrait.narwhals.__all__` listed the classes instead of their names,
  so `from substrait.narwhals import *` raised a TypeError.
- `test_wildcard_match` passed the builtin `bool()` (i.e. `False`) instead of
  the `boolean()` type builder, so it never exercised a boolean argument.

TRY002/TRY004 are ignored rather than fixed: the public API raises plain
`Exception`, and moving to typed exceptions is a separate, user-visible
change. DTZ001 is ignored in tests, where naive datetimes model Substrait's
timezone-less timestamps.
@nielspardon

Copy link
Copy Markdown
Member Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
✅ Action performed

Reviews resumed and review finished.

# Conflicts:
#	pixi.lock
#	tests/extension_registry/test_registry_lookup.py
@nielspardon
nielspardon merged commit 8277940 into substrait-io:main Oct 13, 2026
21 checks passed
@nielspardon
nielspardon deleted the style/ruff-0.16-lint branch October 13, 2026 08:51
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