-
Notifications
You must be signed in to change notification settings - Fork 211
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 more types for Random #470
Conversation
@@ -81,8 +134,9 @@ class Random < Object | |||
# (`-`) and add (`+`)methods, or rand will raise an ArgumentError. | |||
# | |||
def rand: () -> Float | |||
| (Integer | ::Range[Integer] max) -> Integer | |||
| (Integer | ::Range[Integer] | Numeric max) -> Integer |
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.
Seems like having Numeric
here will return Integer
for any numbers including Float
.
Moving this line after next line would give better type.
| (Float | ::Range[Float] max) -> Float | ||
| (::Range[Numeric]) -> Numeric |
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, the type we want would be [T <: Numeric] (Range[T]) -> T
, but RBS currently doesn't allow writing it.
I think there are two options. [T] (Range[T]) -> T
or (Range[Numeric]) -> Numeric
. The first option would make sense in my opinion, because testing the code will uncover the problem anyway.
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.
@zofrex Thanks for opening this pull request! Commented inline.
Add more types for Random (fix conflict #470)
This is my first time writing RBS so I am not sure it is right, especially the tests.
I am not sure how to fully describe
rand
because it can return whatever type you give it, for example:In other words,
rand: (Range[SomeType]) -> SomeType
but I don't know how to write that in RBS.(Also, I think
SomeType
might have to be a descendent ofNumeric
, but the documentation is not clear)