-
Notifications
You must be signed in to change notification settings - Fork 6
fix operator precenedenc, multiple binary operators and enhance tests #56
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
Conversation
…porary fix, should be fixed on the Lark level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes operator precedence handling by adding a preprocessing step, refactors the AST and parser code for cleaner abstraction, and extends tests to cover multiple binary-operator scenarios.
- Add Python 3.13 support in CI and project metadata
- Introduce
_preprocess_expression
to group repeated binary operators correctly - Enhance AST with abstract methods and smarter infix formatting, and expand tests for multiple-operator cases
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
.github/workflows/ci.yml | Bump Python matrix to 3.13, upgrade setup-miniconda and adjust install steps |
pyproject.toml | Add Python 3.13 classifier |
src/formulate/ttreeformula_parser.py | Remove custom error classes, import Lark errors |
src/formulate/AST.py | Convert AST to abstract base class and add formatting methods |
src/formulate/init.py | Add _preprocess_expression for operator grouping |
tests/test_numexpr.py | Add tests for repeated binary operators |
tests/test_failures.py | Add failure tests (empty, invalid, huge, Hypothesis) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes operator precedence issues in expression parsing and enhances test coverage and code maintainability across the codebase. Key changes include:
- Refactoring the AST classes (especially BinaryOperator) to handle multiple binary operators with smart parenthesis removal.
- Enhancing tests for various expression types (TTreeFormula, NumExpr, performance, and failure cases) and updating CI workflows for Python 3.13.
- Cleaning up imports, updating pre-commit configurations, and adding a preprocessing function to improve the parsing reliability of repeated operators.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/test_ttreeformula.py | Updated assertions and cleaned up imports. |
tests/test_root.py | Refined tests with direct string inputs and assertion modifications. |
tests/test_performance.py | Added performance benchmarks for long expressions. |
tests/test_package.py | Removed redundant protocol-based tests. |
tests/test_numexpr.py | Streamlined numexpr tests; adjusted expected outputs. |
tests/test_failures.py | Added robust tests for invalid expressions and edge-case failures. |
src/formulate/toast.py | Minor cleanup; maintains logic for AST conversion via pattern matching. |
src/formulate/matching_tree.py | Consolidated import statements for clarity. |
src/formulate/lark_helpers.py | Introduced fallbacks for Lark exceptions. |
src/formulate/func_translations.py | Improved formatting and spacing. |
src/formulate/convert_ptree.py | Simplified conversion of parse trees with combined imports. |
src/formulate/_utils.py | Refactored string conversion logic with clearer formatting. |
src/formulate/_compat/typing.py | Consolidated typing imports and updated all exports. |
src/formulate/init.py | Added expression preprocessor and streamlined module imports. |
src/formulate/AST.py | Refactored AST class and BinaryOperator for improved operator handling. |
pyproject.toml | Added Python 3.13 classifier and updated dependencies. |
.pre-commit-config.yaml | Updated hook revisions and added new hooks for code formatting and linting. |
.github/workflows/ci.yml | Updated CI configurations for Python 3.13 and updated GitHub Action versions. |
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:45
- [nitpick] Verify that the 'uv' command is intentional and documented, as it is uncommon compared to the typical 'python -m pip install' syntax.
uv pip install --system .[dev,test]
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This pull request introduces several updates, including enhancements to Python compatibility, improvements to the
formulate
library's AST handling, and minor refactoring to streamline code and improve maintainability.Python Compatibility and CI Updates:
.github/workflows/ci.yml
) and updated the classifiers inpyproject.toml
to include Python 3.13. [1] [2]setup-miniconda
GitHub Action from version 2 to version 3 in the CI workflow.uv
androot
packages and optimized the use ofpip
.formulate
Library Enhancements:AST
class to use theABCMeta
metaclass and added abstract methods (__str__
,to_numexpr
,to_root
,to_python
) to enforce implementation in subclasses.BinaryOperator
class with methods for handling complex expressions, smart parenthesis removal, and infix formatting forto_numexpr
,to_root
, andto_python
._preprocess_expression
insrc/formulate/__init__.py
to handle multiple occurrences of the same binary operator in expressions.Codebase Modernization:
# -*- coding: utf-8 -*-
to multiple files for consistency and encoding declaration. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]LarkError
andParseError
definitions with imports from thelark
library innumexpr_parser.py
andttreeformula_parser.py
. [1] [2]Refactoring and Cleanup:
src/formulate/AST.py
,src/formulate/ttreeformula_parser.py
, and other files. [1] [2]ttreeformula_parser.py
. [1] [2]