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

Flexible scientific notation parsing with parse_number() #684

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@sambrady3
Contributor

sambrady3 commented Jun 6, 2017

This is inspired by (but doesn't exactly address) #654
That issue is concerning non-string input of scientific notation, for example: parse_number(2e5)

This PR address string input of scientific notation, for example: parse_number("2e5")
Currently, parse_number() would parse "2e5" as simply 2 because it stops after the first non-numeric encountered in the middle of the number. The scientific notation parsing in this PR maintains much of the flexibility of the current parse_number() (ignoring leading and trailing non-numerics) but deals with the e and E characters specially.

Standard use:

> parse_number("1e20")
[1] 1e+20
> parse_number("3e2")
[1] 300
> parse_number("1.2345e+4") # optional '+' at start of exponent
[1] 12345
> parse_number("-5.4e3")
[1] -5400
> parse_number("17E-5")
[1] 0.00017

Flexibility

> parse_number("ignore17e4ignore")
[1] 170000
> parse_number("-17E-5-5") # ignores characters after second '-', consistent with current use
[1] -0.00017
> parse_number("13e4e5") # ignores additional e's
[1] 130000
@sambrady3

This comment has been minimized.

Contributor

sambrady3 commented Jun 6, 2017

I don't know enough to know why the travis build fails on devel but not release or oldrel.
If sometime down the road maintainers think this is a worthwhile addition, I'd appreciate some help figuring that out. Thanks!

@jimhester

This comment has been minimized.

Member

jimhester commented Dec 6, 2017

Great, thanks! Please add a note to NEWS.md mentioning this change and your GitHub username and I can merge it.

@jimhester

This comment has been minimized.

Member

jimhester commented Dec 7, 2017

I ended up adding the news myself and merged this manually at 0574386, so will close this PR. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment