-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Handle zero-like imaginary part as zero in to_r #9581
Conversation
@@ -1851,8 +1851,11 @@ nucomp_to_r(VALUE self) | |||
get_dat1(self); | |||
|
|||
if (!k_exact_zero_p(dat->imag)) { | |||
rb_raise(rb_eRangeError, "can't convert %"PRIsVALUE" into Rational", | |||
self); | |||
VALUE imag = rb_check_convert_type_with_id(dat->imag, T_RATIONAL, "Rational", idTo_r); |
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.
why convert imag
to rational here just to pass it into k_exact_zero_p()
? Wouldn't it be easier to just change the check k_exact_zero_p(dat->imag)
to f_zero_p(dat->imag)
instead?
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.
@KJTsanaktsidis This approach doesn’t impose the responsibility of deciding what exactly constitutes zero on the Complex class. Instead, it assigns the responsibility to each individual 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.
And this implementation is for proof of concept. Before merging this to the master branch, I’ll add the optimization for Float class later.
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.
Ah that makes sense - thanks for the explanation!
b019354
to
ab737e3
Compare
Waiting for ruby/prism#2468. |
Sorry about the delay @mrkn, you should be able to rebase and it should pass. Please let me know if you run into any more trouble! |
Use imag.to_r when imag is not 0 or 0r. Fixes [Bug ruby#5179]
Use imag.to_r when imag is not 0 or 0r.
Fixes [Bug #5179]