-
Notifications
You must be signed in to change notification settings - Fork 1
Support-old-funz-varform-syntax #59
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
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 pull request adds support for the original Java Funz variable and formula syntax to maintain compatibility with existing Java Funz workflows. The PR introduces several significant features:
Changes:
- Adds support for
()delimiters for variables (e.g.,$(var)) alongside the existing{}syntax (e.g.,${var}) - Implements static object definitions for constants and functions using
#@:prefix syntax - Adds formula evaluation with format specifiers (e.g.,
@{expr | 0.00}) - Introduces multiple model key aliases for backward compatibility (e.g.,
var_prefix/varprefix,formula_prefix/formulaprefix, etc.) - Modifies
fzi()to return a comprehensive dictionary including variables, formulas, and static objects with their values/expressions
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_static_objects.py | Comprehensive tests for static object definitions (constants and functions) with various interpreters |
| tests/test_no_variables.py | Minor variable naming changes (variables→result) for clarity |
| tests/test_model_key_aliases.py | Tests for all model key aliases (var_prefix, formula_prefix, comment) |
| tests/test_java_funz_compatibility.py | Tests verifying Java Funz syntax compatibility |
| tests/test_fzi_formulas.py | Tests for formula parsing and evaluation with default values |
| tests/test_cli_commands.py | Minor comment addition clarifying JSON output structure |
| fz/interpreter.py | Core implementation of parsing logic, formula evaluation, and static object handling |
| fz/helpers.py | Updates to use new model key aliases with backward compatibility |
| fz/core.py | Major changes to fzi() function and addition of helper functions for model key resolution |
| examples/java_funz_syntax_example.py | Example demonstrating Java Funz syntax usage |
| examples/fzi_static_objects_example.py | Example showing static object definitions |
| examples/fzi_formulas_example.py | Example demonstrating formula parsing and evaluation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fz/core.py
Outdated
| def _get_comment_char(model: Dict) -> str: | ||
| """ | ||
| Get comment character from model with support for multiple aliases | ||
|
|
||
| Supported keys (in order of precedence): | ||
| - commentline | ||
| - comment_line | ||
| - comment_char | ||
| - commentchar | ||
| - comment | ||
|
|
||
| Args: | ||
| model: Model definition dict | ||
|
|
||
| Returns: | ||
| Comment character (default "#") | ||
| """ | ||
| return model.get( | ||
| "commentline", | ||
| model.get( | ||
| "comment_line", | ||
| model.get( | ||
| "comment_char", | ||
| model.get( | ||
| "commentchar", | ||
| model.get("comment", "#") | ||
| ) | ||
| ) | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| def _get_var_prefix(model: Dict) -> str: | ||
| """ | ||
| Get variable prefix from model with support for multiple aliases | ||
|
|
||
| Supported keys (in order of precedence): | ||
| - var_prefix | ||
| - varprefix | ||
| - var_char | ||
| - varchar | ||
|
|
||
| Args: | ||
| model: Model definition dict | ||
|
|
||
| Returns: | ||
| Variable prefix (default "$") | ||
| """ | ||
| return model.get( | ||
| "var_prefix", | ||
| model.get( | ||
| "varprefix", | ||
| model.get( | ||
| "var_char", | ||
| model.get("varchar", "$") | ||
| ) | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| def _get_formula_prefix(model: Dict) -> str: | ||
| """ | ||
| Get formula prefix from model with support for multiple aliases | ||
|
|
||
| Supported keys (in order of precedence): | ||
| - formula_prefix | ||
| - formulaprefix | ||
| - form_prefix | ||
| - formprefix | ||
| - formula_char | ||
| - form_char | ||
|
|
||
| Args: | ||
| model: Model definition dict | ||
|
|
||
| Returns: | ||
| Formula prefix (default "@") | ||
| """ | ||
| return model.get( | ||
| "formula_prefix", | ||
| model.get( | ||
| "formulaprefix", | ||
| model.get( | ||
| "form_prefix", | ||
| model.get( | ||
| "formprefix", | ||
| model.get( | ||
| "formula_char", | ||
| model.get("form_char", "@") | ||
| ) | ||
| ) | ||
| ) | ||
| ) | ||
| ) |
Copilot
AI
Jan 24, 2026
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.
These three helper functions (_get_comment_char, _get_var_prefix, _get_formula_prefix) are duplicated from fz/interpreter.py. This violates the DRY (Don't Repeat Yourself) principle and creates maintenance issues. If the logic needs to change, it must be updated in both places. Consider importing these functions from interpreter.py instead of redefining them here.
fz/interpreter.py
Outdated
| if '=' in line and not line.startswith('def ') and '<-' not in line: | ||
| parts = line.split('=', 1) | ||
| if len(parts) == 2: | ||
| name = parts[0].strip() | ||
| # Remove any type hints | ||
| if ':' in name: | ||
| name = name.split(':')[0].strip() | ||
| value = parts[1].strip() | ||
| expressions[name] = value | ||
| i += 1 | ||
| continue |
Copilot
AI
Jan 24, 2026
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.
This check for Python assignment may incorrectly match comparison operators. For example, a line like "if x == value:" contains '=' but should not be treated as an assignment. The check should use a more robust pattern, such as checking for '==' or other comparison operators first, or using a proper AST parser. Consider using regex to match assignment patterns more precisely: r'^\s*([a-zA-Z_][a-zA-Z0-9_])\s=\s*(?!=)' to avoid matching '==', '<=', '>=', '!=', etc.
fz/core.py
Outdated
| # Try numeric conversion | ||
| if '.' in default_value or 'e' in default_value.lower(): | ||
| variable_defaults[var_name] = float(default_value) | ||
| else: | ||
| variable_defaults[var_name] = int(default_value) | ||
| except ValueError: |
Copilot
AI
Jan 24, 2026
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.
The numeric conversion logic may fail for certain valid numeric formats. For example, hexadecimal numbers (0x1F), octal numbers (0o77), binary numbers (0b1010), or numbers with underscores (1_000_000) are valid in Python but won't be handled correctly here. Consider using ast.literal_eval() for more robust parsing, which can handle various Python literal formats safely.
| # Try numeric conversion | |
| if '.' in default_value or 'e' in default_value.lower(): | |
| variable_defaults[var_name] = float(default_value) | |
| else: | |
| variable_defaults[var_name] = int(default_value) | |
| except ValueError: | |
| # Try numeric conversion using ast.literal_eval to support | |
| # various Python numeric literal formats (hex, octal, binary, | |
| # underscores, etc.). | |
| evaluated = ast.literal_eval(default_value) | |
| if isinstance(evaluated, (int, float)): | |
| variable_defaults[var_name] = evaluated | |
| else: | |
| # Not a numeric literal; fall through to string handling | |
| raise ValueError | |
| except (ValueError, SyntaxError): |
| dedented_code = textwrap.dedent(full_code) | ||
|
|
||
| try: | ||
| exec(dedented_code, env) |
Copilot
AI
Jan 24, 2026
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.
Using exec() to evaluate arbitrary user code from input files poses a significant security risk. If an attacker can control the input file content, they can execute arbitrary Python code on the system. While this may be acceptable for trusted input files in certain use cases, consider adding a warning in the documentation about this security implication, or implementing a sandboxed execution environment. At minimum, document that input files should only come from trusted sources.
| # Extract context lines from model if available | ||
| context_lines = [] | ||
|
|
Copilot
AI
Jan 24, 2026
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.
Variable context_lines is not used.
| # Extract context lines from model if available | |
| context_lines = [] |
|
|
||
| from .logging import log_error, log_warning, log_info, log_debug, log_progress | ||
| from .config import get_config | ||
| from .config import get_config, get_interpreter |
Copilot
AI
Jan 24, 2026
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.
Import of 'get_config' is not used.
| from .config import get_config, get_interpreter | |
| from .config import get_interpreter |
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>
No description provided.