Skip to content
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

Make the type of value in NumericValue actually a number #123

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DillonZChen
Copy link

Proposed changes

Automatically convert the value constructor variable of NumericValue into an actual Python number (float or int) as specified in the type hinting. The current parser in the code inputs a lark.lexer.Token when initialising an NumericValue.

Fixes

Added a quick type conversion in the init code and added corresponding test to check that calling the value property indeed returns a float.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING doc
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works

Further comments

Using just float(value) breaks test_numerical_hello_world_domain_formatter test because it converts ints to floats, hence why I added some more code to preserve ints where possible. It's probably possible to go down a rabbit hole on specifying different types of numbers such as involving Rationals in order to preserve numerical accuracy, so I'm not sure what is best to do here.

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