-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Documentation: BigFloat #5639
Documentation: BigFloat #5639
Conversation
src/big/big_float.cr
Outdated
@@ -217,11 +243,21 @@ struct BigFloat < Float | |||
mpf | |||
end | |||
|
|||
# Returns an inspected representation of self. |
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.
self => self
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.
👍
src/big/big_float.cr
Outdated
def inspect(io) | ||
to_s(io) | ||
io << "_big_f" | ||
end | ||
|
||
# Returns a string representation of self. |
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.
ditto
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.
Thanks Sija. Fixed!
# Raises `ArgumentError` if the string doesn't denote a valid float. | ||
# | ||
# ``` | ||
# BigFloat.new("-0.123456789101101987654321") # => -0.123456789101101987654321 |
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.
_big_f
suffix?
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.
nope, it's added only in #inspect
.
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.
Yeah, that's what the expression value should show, see the example of the arg-less constructor above.
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.
@straight-shoota Don't you like BigFloat.new.inspect # => 0_big_f
?
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'm not sure if I like it in general, because it looks like a literal but there is none.
However, if you use that format for the value of the arg-less constructor example, this one should be equal.
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.
Another reason for removing suffixes... there are none for big numbers. Unless we inspect them to BigInt.new("#{string}")
but that's unreadable and ugly.
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.
Good points - fixed!
Is there anything else to be done here? |
src/big/big_float.cr
Outdated
@@ -217,11 +243,21 @@ struct BigFloat < Float | |||
mpf | |||
end | |||
|
|||
# Returns an inspected representation of `self`. |
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.
Returns a human-readable representation of self.
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 👍
src/big/big_float.cr
Outdated
def inspect(io) | ||
to_s(io) | ||
io << "_big_f" | ||
end | ||
|
||
# Returns a string representation of `self`. |
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.
Specify base 10.
We really should support other bases here, but thats another issue.
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.
Would you like a # TODO: support other bases
here ?
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.
@RX14 Actually on reflection does it really make much sense in talking about float representation in anything other than base 10, and by extension should I maybe revert the change to specify base 10 ? Certainly there doesn't appear to be any use or mention of alternate bases in Float32 / Float64 for instance. Perhaps you were thinking of BigInt?
+ # Returns a string representation (base 10) of `self`.
+ #
+ # ```
+ # BigFloat.new("0.123456789101101987654321").to_s # => 0.123456789101101987654321
+ # ```
def to_s(io : IO)
Specified base 10, rebased and squashed |
Is there anything else to be done here? |
@mjago lgtm @RX14 @straight-shoota is this OK? |
Please don't close things due to inactivity. Especially because you've deleted the original repo so I cannot reopen this PR. |
Documentation for BigFloat