Bug #15857
closed<=> の右辺が <=> を実装していない場合の振る舞い
Added by shuujii (Shuji KOBAYASHI) over 5 years ago. Updated almost 4 years ago.
Description
<=>
の右辺が <=>
を実装していないとき、nil
が返却される場合と例外が発生する場合があり一貫性がないように思えるのですが、意図的でしょうか。
0 <=> 0i #=> NoMethodError (undefined method `<=>' for (0+0i):Complex)
0 <=> BasicObject.new #=> nil
:a <=> 0i #=> nil
"a" <=> 0i #=> NoMethodError (undefined method `<=>' for (0+0i):Complex)
なお、0 <=> 0i
に関しては、0 == 0i
は true
になるのでそれとも一貫性がないように思えるのもやや気になりました。
Files
complex-real-spaceship.patch (8.75 KB) complex-real-spaceship.patch | jeremyevans0 (Jeremy Evans), 06/05/2019 04:53 AM | ||
complex-real-spaceship-v2.patch (8.08 KB) complex-real-spaceship-v2.patch | jeremyevans0 (Jeremy Evans), 06/06/2019 02:32 AM | ||
complex-real-spaceship-v3.patch (7.79 KB) complex-real-spaceship-v3.patch | jeremyevans0 (Jeremy Evans), 06/11/2019 05:14 PM |
Updated by jeremyevans0 (Jeremy Evans) over 5 years ago
I agree that this is a bug. Complex#<=>
should be implemented, even if it did return nil
for all arguments.
However, I think Complex numbers with an imaginary part of zero should be treated as real numbers, and that Complex#real?
should return true
for such numbers. Likewise, Complex#<=>
should return 1, 0, or -1 if both the receiver and the argument are real numbers (treating Complex instances with an imaginary part of zero as real numbers). However, if the receiver or the argument is not a real number, then it should return nil
.
Attached is a patch that implements the behavior described above.
Updated by sawa (Tsuyoshi Sawada) over 5 years ago
jeremyevans0 (Jeremy Evans) wrote:
I think [that] Complex numbers with an imaginary part of zero should be treated as real numbers, and that
Complex#real?
should returntrue
for such numbers.
Changing Complex#real?
's meaning has huge influence. You would also have to change Numeric#integer?
for consistency.
Notice that in Ruby, there is no class Real
, and real?
means "not an instance of Complex
". I believe Float
is an approximation of the concept of real numbers in mathematics, but unfortunately, Integer
and Rational
are not subclasses of Float
like integers and rational numbers are subsets of real numbers in the mathematical sense. So discussing your proposal in terms of real numbers does not look that helpful.
Complex#<=>
should return 1, 0, or -1 if both the receiver and the argument are real numbers (treating Complex instances with an imaginary part of zero as real numbers).
I think this should rather be handled by coercion as with arithmetic operations.
Updated by jeremyevans0 (Jeremy Evans) over 5 years ago
sawa (Tsuyoshi Sawada) wrote:
jeremyevans0 (Jeremy Evans) wrote:
I think [that] Complex numbers with an imaginary part of zero should be treated as real numbers, and that
Complex#real?
should returntrue
for such numbers.Changing
Complex#real?
's meaning has huge influence. You would also have to changeNumeric#integer?
for consistency.Notice that in Ruby, there is no class
Real
, andreal?
means "not an instance ofComplex
". I believeFloat
is an approximation of the concept of real numbers in mathematics, but unfortunately,Integer
andRational
are not subclasses ofFloat
like integers and rational numbers are subsets of real numbers in the mathematical sense. So discussing your proposal in terms of real numbers does not look that helpful.
Thank you for providing some background on the purpose of the real?
method. I agree that it doesn't make sense to change real?
. Attached is a patch that only implements <=>
and removes the modifications to real?
. It also adds specs for <=>
.
Complex#<=>
should return 1, 0, or -1 if both the receiver and the argument are real numbers (treating Complex instances with an imaginary part of zero as real numbers).I think this should rather be handled by coercion as with arithmetic operations.
Could you please provide a patch for your approach so we can compare and choose the simpler implementation?
Updated by Eregon (Benoit Daloze) over 5 years ago
The patch looks good to me.
Updated by shuujii (Shuji KOBAYASHI) over 5 years ago
@jeremyevans0 (Jeremy Evans) Thank you for your comment. About Complex
, I agree with most of v2 patch, but I have some comments.
- Should methods from
Comparable
that are currently disabled be enabled? -
rb_undef_method(rb_cComplex, "<=>")
is unneeded. -
idCmp
can be used instead ofid_spaceship
.
About other than Complex
, <=>
of some classes, such as String
and Time
, will fail if LHSRHS does not implement <=>
. It's better to return nil
in this case too, because whether it's comparable or not and whether LHSRHS implements <=>
or not is essentially irrelevant, I think.
Updated by jeremyevans0 (Jeremy Evans) over 5 years ago
shuujii (Shuji KOBAYASHI) wrote:
@jeremyevans0 (Jeremy Evans) Thank you for your comment. About
Complex
, I agree with most of v2 patch, but I have some comments.
- Should methods from
Comparable
that are currently disabled be enabled?
I don't think so, but I don't have a strong opinion. If you would like those added, please submit a separate feature request for that.
rb_undef_method(rb_cComplex, "<=>")
is unneeded.idCmp
can be used instead ofid_spaceship
.
Thank you, I made both of these changes in the attached v3 patch.
About other than
Complex
,<=>
of some classes, such asString
andTime
, will fail if LHS does not implement<=>
. It's better to returnnil
in this case too, because whether it's comparable or not and whether LHS implements<=>
or not is essentially irrelevant, I think.
I agree, but please submit a separate bug report for that as it is unrelated to Complex#<=>
.
Updated by shuujii (Shuji KOBAYASHI) over 5 years ago
@jeremyevans0 (Jeremy Evans) Thank you for your comment. About Complex, I agree with most of v2 patch, but I have some comments.
- Should methods from Comparable that are currently disabled be enabled?
I don't think so, but I don't have a strong opinion. If you would like those added, please submit a separate feature request for that.
I don't have a strong opinion, too.
About other than Complex, <=> of some classes, such as String and Time, will fail if
LHSRHS does not implement <=>. It's better to return nil in this case too, because whether it's comparable or not and whetherLHSRHS implements <=> or not is essentially irrelevant, I think.I agree, but please submit a separate bug report for that as it is unrelated to Complex#<=>.
Sure. I will create another ticket.
Updated by jeremyevans (Jeremy Evans) over 5 years ago
- Status changed from Open to Closed
Applied in changeset git|b9ef35e4c6325864e013ab6e45df6fe00f759a47.
Implement Complex#<=>
Implement Complex#<=> so that it is usable as an argument when
calling <=> on objects of other classes (since #coerce will coerce
such numbers to Complex). If the complex number has a zero imaginary
part, and the other argument is a real number (or complex number with
zero imaginary part), return -1, 0, or 1. Otherwise, return nil,
indicating the objects are not comparable.
Fixes [Bug #15857]
Updated by kolbrich (Kevin Olbrich) almost 4 years ago
In doing a bit of maintenance work for the ruby-units gem this issue came up. I think there may be some additional methods that now need to be generated in the C code. See the following example.
c = Complex(1,0)
c <=> 1 # => 0
c > 1 # => NoMethodError (undefined method `>' for (1+0i):Complex)
c < 1 # => NoMethodError (undefined method `<' for (1+0i):Complex)
c <= 1 # => NoMethodError (undefined method `<=' for (1+0i):Complex)
c >= 1 # => NoMethodError (undefined method `>=' for (1+0i):Complex)
I would expect these methods to exist and behave in a way that is consistent with the new definition of <=>.