-
Notifications
You must be signed in to change notification settings - Fork 326
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 vectorized rounding operation to Decimal columns #10912
Conversation
import org.enso.table.error.UnexpectedTypeException; | ||
import org.graalvm.polyglot.Context; | ||
|
||
/** An operation rounding floating-point numbers. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment seems to be misaligned with the class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
for (int i = 0; i < storage.size(); i++) { | ||
if (!storage.isNothing(i)) { | ||
BigDecimal value = storage.getItem(i); | ||
BigDecimal result = Decimal_Utils.round(value, (int) decimalPlaces.longValue(), useBankers); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that decimalPlaces
will not overflow int
because check_decimal_places
ensures it is between [-15; 15]. Still I think it would be safer to include an assert
here to ensure that the overflow does not happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just some minor nits
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.