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

Let Complex inherit from Number #5436

Closed
wants to merge 1 commit into from

Conversation

Sija
Copy link
Contributor

@Sija Sija commented Dec 21, 2017

While digging through stdlib I've noticed that Complex for some reason does not subclass Number.
It seems like an omission, since it is a number. This PR changes that.

Also, in Ruby Complex < Numeric # => true.

@larubujo
Copy link
Contributor

Number has methods:

  • step
  • sign
  • <=>
  • significant
  • round
  • clamp

not make sense for complex

@larubujo
Copy link
Contributor

see why inheritance makes things complex?

(pun intended)

@Sija Sija mentioned this pull request Dec 22, 2017
@yxhuvud
Copy link
Contributor

yxhuvud commented Dec 23, 2017

Uh, how does step not make sense in a complex setting? It is quite possible to step even in a complex space, but an implementation may want to check that the points is parallel to the line that the step defines.

significant and round makes total sense and would operate on each axis independently. All three would require complex specific implementations (that at least in significant and rounds' case would make heavy use of the base implementation.

@RX14
Copy link
Contributor

RX14 commented Dec 24, 2017

@yxhuvud I agree that step somewhat makes sense but as you said, the method gets a lot more complex in 2d.

Please: let's discuss if we want to change the Number interface to not guarantee an ordering (I disagree on this change) in another issue, this PR can't be merged until that is agreed on.

@straight-shoota
Copy link
Member

Closing because it's stalled for bigger scoped changes to Number. The implementation is more than trivial, so not need to carry this PR around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants