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

A more robust parse method #21

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

lcontento
Copy link

The current implementation of the parse method for the Decimal type cannot handle correctly cases such as "1.0000001e6", "30e-2", "0.1234567e-15" or "123456789.1234567899e-11".
I wrote a replacement proposal that handles these cases. It does not treat scientific notation as a separate case, so the function scinote (and the parameters functions) are no longer used.

Cases such as 1.0000001e6 are now handled correctly.
scinote and parameters functions are no longer used.
@lcontento
Copy link
Author

I had forgotten to deal with the simplest case possible ("0.0"). Now it should work.
Moreover, now "-0.0" is parsed as Decimal(1, 0, 0) instead of Decimal(1, 0, -1)

src/decimal.jl Outdated
# Read sign
s = (str[1] == '-') ? 1 : 0
# Unpack scientific notation
scno = split(str, 'e')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO longer / more descriptive variable names would be really nice, although I know this repo is bad about this in general (mea culpa...).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried making the variable names more suggestive. I also added checks for excluding double occurrences of '.' or 'e' in the string.
About the exponent separator for scientific notation, originally it was just 'e', but I believe that also 'E' should be allowed since I've seen many tabulated data using it. I just added a call to lowercase at the beginning for this. If you do not agree I can revert it.

* More suggestive variable names
* Allow 'E' as a exponent character
* Raise an error if '.' or 'e' appear twice in the string to be parsed
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.

None yet

2 participants