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 Text.to_decimal #10874

Merged
merged 6 commits into from
Aug 28, 2024
Merged

Add Text.to_decimal #10874

merged 6 commits into from
Aug 28, 2024

Conversation

GregoryTravis
Copy link
Contributor

@GregoryTravis GregoryTravis commented Aug 22, 2024

Pull Request Description

Add Text.to_decimal.
Also makes renames Decimal.with_scale to set_scale and makes it public.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.

Convert this `Text` to a `Decimal`.

Arguments:
- scale: the optional Decimal scale to use. See `Decimal.set_scale` for more
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have a way to link directly to other parts of our documentation?

Copy link
Member

Choose a reason for hiding this comment

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

Not really at the moment.

We should use the Markdown syntax and could pick up in the website version and then think about adding to the doc panel as well.

@GregoryTravis GregoryTravis marked this pull request as ready for review August 22, 2024 18:33
Convert this `Text` to a `Decimal`.

Arguments:
- scale: the optional Decimal scale to use. See `Decimal.set_scale` for more
Copy link
Member

Choose a reason for hiding this comment

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

Not really at the moment.

We should use the Markdown syntax and could pick up in the website version and then think about adding to the doc panel as well.

Comment on lines +1161 to +1206
## ADVANCED
GROUP Math
ICON math
Change the scale of a Decimal.

A `Decimal` value is represented internally by a Java `BigInteger` "unscaled
value" and a "scale value". The numerical value of the `Decimal` is
`(unscaledValue * 10^(-scale))`. Scale values are maintained automatically by
the constructors and numerical operations, but can also be set explicitly.

Scale values can allow distinctions between values that would be identical as
`Float`s. For example, the following values have different internal
representations:

a = Decimal.new "2.0"
b = Decimal.new "2.00"
a == b
# => True

These two values have different internal representations, but they are still
considered the same value by `==`.

! Error Conditions

- If an explicit `scale` parameter is passed, and the scale is not
large enough to represent the number exactly, an `Arithmetic_Error`
is thrown.

> Example
Set the scale of a `Decimal`

d = dec "23.456" set_scale 4
d.internal_representation
# => [234560, 6, 4]

> Example
Get an error when using a scale that is too small.
(A scale of 2 is not enough to represent three decimal places.)

dec "23.456" set_scale 2
# => Arithmetic_Error
set_scale : Integer -> Decimal
set_scale self new_scale:Integer -> Decimal =
handle_java_exception <|
if self.scale == new_scale then self else
Decimal.Value (self.big_decimal.setScale new_scale)
Copy link
Member

Choose a reason for hiding this comment

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

Curious: what is the use-case for making set_scale public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since Text.to_decimal exposes the scale parameter, it makes sense to be consistent and expose this as well.

@GregoryTravis GregoryTravis added the CI: Ready to merge This PR is eligible for automatic merge label Aug 26, 2024
@GregoryTravis GregoryTravis added CI: Ready to merge This PR is eligible for automatic merge and removed CI: Ready to merge This PR is eligible for automatic merge labels Aug 28, 2024
@mergify mergify bot merged commit 5fba572 into develop Aug 28, 2024
35 checks passed
@mergify mergify bot deleted the wip/gmt/10427-text-to-decimal branch August 28, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants