Conversation
There was a problem hiding this comment.
Pull request overview
This PR applies Ruff linting rule fixes, primarily focused on simplifying code style, improving type annotations, and fixing a critical version check bug. The changes improve code quality and maintainability while preserving functionality.
Changes:
- Renamed
Setimports toAbstractSetthroughout the codebase to avoid shadowing the builtinsettype - Fixed inverted version condition in
pymbolic/interop/ast.pyfor Python 3.14 compatibility - Improved type annotations using
Neverreturn type for functions that unconditionally raise exceptions - Simplified code by removing unnecessary intermediate variables, empty
passstatements, and redundantelseblocks - Standardized imports (e.g.,
from pytools import obj_arrayinstead ofimport pytools.obj_array as obj_array) - Optimized loops and replaced with comprehensions where appropriate
- Updated linting configuration to ignore S102 (exec) and TRY004
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/test_pymbolic.py | Renamed Set to AbstractSet in imports and type hints |
| test/test_maxima.py | Simplified print("") to print() |
| pyproject.toml | Added S102 and TRY004 to lint ignore list, expanded per-file ignores |
| pymbolic/rational.py | Standardized imports to use from pymbolic import |
| pymbolic/primitives.py | Multiple improvements: import style, re.VERBOSE, removed global declarations, removed pass statements, added noqa comments |
| pymbolic/mapper/substitutor.py | Renamed Set to AbstractSet |
| pymbolic/mapper/optimize.py | Added noqa comment for exec usage |
| pymbolic/mapper/distributor.py | Removed unnecessary intermediate variable |
| pymbolic/mapper/dependency.py | Renamed Set to AbstractSet |
| pymbolic/mapper/collector.py | Renamed Set to AbstractSet, removed intermediate variable |
| pymbolic/mapper/coefficient.py | Improved loop to use items() instead of repeated dict lookups |
| pymbolic/mapper/init.py | Renamed Set to AbstractSet, removed unnecessary else blocks, improved loop efficiency |
| pymbolic/interop/sympy.py | Changed return type to Never for error function |
| pymbolic/interop/symengine.py | Changed return type to Never for error function |
| pymbolic/interop/common.py | Made error function abstract with Never return type, removed unreachable raise statements |
| pymbolic/interop/ast.py | CRITICAL: Fixed inverted version condition for Python 3.14 compatibility |
| pymbolic/imperative/utils.py | Optimized loops to use dict.items(), converted loop to comprehension |
| pymbolic/imperative/transform.py | Converted append loop to extend with comprehension |
| pymbolic/imperative/statement.py | Renamed Set to AbstractSet, removed intermediate variables |
| pymbolic/geometric_algebra/primitives.py | Standardized import of obj_array |
| pymbolic/geometric_algebra/mapper.py | Renamed Set to AbstractSet, standardized obj_array import |
| pymbolic/geometric_algebra/init.py | Improved Protocol type hints using Self, converted loops to comprehensions |
| pymbolic/cse.py | Renamed Set to AbstractSet, removed intermediate variable |
| pymbolic/algorithm.py | Simplified range(0, m) to range(m), added Self import, standardized import |
| .basedpyright/baseline.json | Updated type checker baseline after code changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #238.