-
Notifications
You must be signed in to change notification settings - Fork 1
Explicit parenthesis around exponent values #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
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 implements explicit parenthesis around exponent values in Parslet parsing and transformation rules, replacing string-based exponent handling with a new Number class. The changes introduce structured handling of power numerators by wrapping exponent values in a Number object, updating conversion methods to support the new structure, and modifying test expectations to align with the new data structures.
- Introduces a new Number class to handle exponent values with proper conversion methods
- Updates Unit, Dimension, and other classes to use Number objects for power_numerator instead of strings
- Modifies parsing rules to structure exponent values as power_numerator objects with explicit parenthesis support
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/unitsml/parser_spec.rb | Updates test expectations to use Number objects for exponents |
| spec/unitsml/parse_spec.rb | Modifies parser test assertions to check power_numerator structure |
| spec/unitsml/conv/xml_spec.rb | Updates XML conversion test expectations and removes extra mrow tags |
| spec/unitsml/conv/mathml_spec.rb | Removes unnecessary mrow wrappers in MathML output expectations |
| lib/unitsml/version.rb | Adds freeze to version string |
| lib/unitsml/utility.rb | Refactors utility methods to work with Number objects and improves code quality |
| lib/unitsml/unitsdb/*.rb | Updates registration method calls to use module methods |
| lib/unitsml/unit.rb | Replaces string-based exponent handling with Number objects and updates conversion methods |
| lib/unitsml/transform.rb | Updates transformation rules to use power_numerator instead of integer |
| lib/unitsml/parser.rb | Modifies unit exponent handling to use Number objects |
| lib/unitsml/parse.rb | Updates parsing rule to wrap exponents in power_numerator |
| lib/unitsml/number.rb | New class implementing structured number handling with conversion methods |
| lib/unitsml/intermediate_exp_rules.rb | Adds support for explicit parenthesis in exponent parsing |
| lib/unitsml/formula.rb | Updates to use Number objects for dimensions |
| lib/unitsml/fenced.rb | Adds bridge methods for Number compatibility |
| lib/unitsml/dimension.rb | Updates dimension handling to use Number objects |
| lib/unitsml.rb | Adds module methods for registration and type substitution |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lib/unitsml/fenced.rb
Outdated
| # A bridge method to check if the Number instance's value is negative. | ||
| def negative? | ||
| value.negative? | ||
| end |
Copilot
AI
Sep 22, 2025
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.
[nitpick] These bridge methods in the Fenced class create tight coupling with the Number class. Consider using composition patterns or mixins to avoid duplicating Number's interface methods.
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
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
lib/unitsml/utility.rb:1
- Magic string 'unknown' is used directly. This should use the
UNKNOWNconstant defined at line 47 for consistency.
# frozen_string_literal: true
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Time | ||
| ].freeze | ||
|
|
||
| UNKNOWN = "unknown" |
Copilot
AI
Sep 22, 2025
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.
Magic string 'unknown' is used directly. This should use the UNKNOWN constant defined at line 47 for consistency.
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
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
parenthesis around exponent values
deb1130 to
94fd263
Compare
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
Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Thank you @suleman-uzair ! |
This PR updates Parslet parsing and transformation rules and the conversion methods.
closes #46