Skip to content

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

Merged
merged 17 commits into from
May 16, 2025

Conversation

jonas-eschle
Copy link
Contributor

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:

  • Added support for Python 3.13 in the CI workflow (.github/workflows/ci.yml) and updated the classifiers in pyproject.toml to include Python 3.13. [1] [2]
  • Upgraded the setup-miniconda GitHub Action from version 2 to version 3 in the CI workflow.
  • Modified the installation process for test dependencies in the CI workflow to include uv and root packages and optimized the use of pip.

formulate Library Enhancements:

  • Refactored the AST class to use the ABCMeta metaclass and added abstract methods (__str__, to_numexpr, to_root, to_python) to enforce implementation in subclasses.
  • Enhanced the BinaryOperator class with methods for handling complex expressions, smart parenthesis removal, and infix formatting for to_numexpr, to_root, and to_python.
  • Introduced a preprocessing function _preprocess_expression in src/formulate/__init__.py to handle multiple occurrences of the same binary operator in expressions.

Codebase Modernization:

  • Added # -*- 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]
  • Replaced custom LarkError and ParseError definitions with imports from the lark library in numexpr_parser.py and ttreeformula_parser.py. [1] [2]

Refactoring and Cleanup:

  • Removed redundant imports and streamlined code in src/formulate/AST.py, src/formulate/ttreeformula_parser.py, and other files. [1] [2]
  • Added new utility functions for deep copying and grammar handling in ttreeformula_parser.py. [1] [2]

@jonas-eschle jonas-eschle requested a review from Copilot May 15, 2025 15:21
Copy link
Contributor

@Copilot Copilot AI left a 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)

jonas-eschle and others added 8 commits May 15, 2025 17:27
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>
@jonas-eschle jonas-eschle requested a review from Copilot May 16, 2025 09:53
Copy link
Contributor

@Copilot Copilot AI left a 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]

jonas-eschle and others added 2 commits May 16, 2025 14:54
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jonas-eschle jonas-eschle enabled auto-merge May 16, 2025 12:57
@jonas-eschle jonas-eschle merged commit 95e38f2 into main May 16, 2025
4 of 5 checks passed
@jonas-eschle jonas-eschle deleted the fix/multiple_operators branch May 16, 2025 13:01
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