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 abs, round, floor, ceil operators #6496

Merged
merged 5 commits into from
Apr 11, 2018
Merged

Add abs, round, floor, ceil operators #6496

merged 5 commits into from
Apr 11, 2018

Conversation

anandthakker
Copy link
Contributor

@anandthakker anandthakker commented Apr 11, 2018

Refs #5853

TODO:

  • Round away from 0 instead of towards +Infinity for halfway values.
  • Add doc entries and SDK support entries to v8.json

@anandthakker
Copy link
Contributor Author

"ceil" matches Javascript's Math.ceil -- should we use "ceiling" instead?

@anandthakker
Copy link
Contributor Author

cc @samanpwbb

Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

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

It's ceil in C, C++, JavaScript, Python, and Java so let's stick with that.

'abs': [
NumberType,
[NumberType],
(ctx, args) => Math.abs(args[0].evaluate(ctx))
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer argument destructuring as in the other definitions, i.e. (ctx, [n]) => ....

@@ -2490,6 +2490,42 @@
}
}
},
"round": {
"doc": "Rounds the input to the nearest integer. Halflway values are rounded away from zero. E.g., `[\"round\", -1.5]` evaluates to -2.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Halflway → Halfway

@@ -2490,6 +2490,42 @@
}
}
},
"round": {
"doc": "Rounds the input to the nearest integer. Halflway values are rounded away from zero. E.g., `[\"round\", -1.5]` evaluates to -2.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: It’s somewhat unusual to begin a sentence with “E.g.” “For example” is more easily understood.

}
},
"abs": {
"doc": "Returns the absolute value of the input.",
Copy link
Member

Choose a reason for hiding this comment

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

when implementing I noticed abs on -1.1 returns 1, could we include in the documentation that abs is an integer type function/not floating point? (from a java background this was a bit confusing for me).

Copy link
Member

Choose a reason for hiding this comment

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

capturing from chat that above assumption was invalid

@anandthakker
Copy link
Contributor Author

anandthakker commented Apr 11, 2018 via email

@anandthakker anandthakker merged commit 6b96d69 into master Apr 11, 2018
@anandthakker anandthakker deleted the rounding branch April 11, 2018 15:18
anandthakker added a commit to mapbox/mapbox-gl-native that referenced this pull request Apr 11, 2018
* Add abs, round, floor, ceil operators

Port of mapbox/mapbox-gl-js#6496

* [ios, macos] Simplified abs, ceiling, floor expressions

* [ios, macos] Added rounding expression function

* [android] - binding integration for round, ceil, floor and abs expressions

* Update mapbox-gl-js to include non-integer rounding test

* Drop extra braces

* mapbox-gl-js -> master

* Update style-spec docs -> PropertyFactory.java
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

4 participants