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

Add is-nan expression #5494

Closed
anandthakker opened this issue Oct 20, 2017 · 6 comments
Closed

Add is-nan expression #5494

anandthakker opened this issue Oct 20, 2017 · 6 comments

Comments

@anandthakker
Copy link
Contributor

We currently don't have very robust NaN handling in expression evaluation. For instance, ["typeof", ["^", -1, 0.5]] yields "number", even though the ^ expression produces a NaN.

@jfirebaugh
Copy link
Contributor

For instance, ["typeof", ["^", -1, 0.5]] yields "number", even though the ^ expression produces a NaN.

I know people make fun of Javascript for typeof NaN === "number", but it's the correct semantics. NaN is an instance of the number type -- just a special one. Treating it differently would be more awkward.

@anandthakker
Copy link
Contributor Author

Keeping NaN has its own awkwardness... e.g., when to-number fails, is it an error, or NaN? If to-number is used with fallbacks, does it stop at NaN or proceed to a fallback - i.e., what does ["to-number", "not a number", 10] return?

If we are going to allow NaN, though, we should add an is-nan expression.

@jfirebaugh
Copy link
Contributor

when to-number fails, is it an error, or NaN

An error, like other conversion expressions.

["to-number", "not a number", 10] => 10
["to-number", ["^", -1, 0.5], 10] => NaN

we should add an is-nan expression

👍

@jfirebaugh jfirebaugh changed the title Handle NaN values in expression evaluation Add is-nan expression Oct 25, 2017
@anandthakker
Copy link
Contributor Author

Along with adding is-nan, we should add tests to make sure that all number-accepting expressions do something reasonable when they get a NaN (e.g., curve needs to behave reasonably when its input is NaN).

@jfirebaugh
Copy link
Contributor

Related: #4172.

@ryanhamley
Copy link
Contributor

I'm closing this as not necessary anymore since we added NaN handling in expression validation with #8615

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

No branches or pull requests

3 participants