Numeric fixes #2176
Merged
Numeric fixes #2176
+211
−60
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
into fixnums or bignums, and raises an exception if it is unable to do so.
… according to MRI behavior.
Including support for kwargs and better infinite bounds handling. To support this change, ArgsUtil.extractKeywordArgs has been added which will validate kwargs and extract an array of values given a list of keywords to accept. This should probably be rolled into the @JRubyMethod decorator at some point so that kwargs validation and extraction is transparent.
|
Looks good. One comment: if a given method or block of code is clearly trackable back to something in MRI, try to add a comment above the method or block indicating where it came from like Thanks! The Numeric#step logic looks like it gained a lot of missing functionality. |
|
Will do, thanks. I rewrote that code probably twelve times yesterday, so it doesn't surprise me that I missed a few places. :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Un-exclude most tests in test_numeric and test_fixnum.
bitCoerceupdated to use the newNumeric.doCoercecoercionNumeric#stepupdated to accept kwargs and to enforce legacy step requirements.Numeric#quoproperly casts to rationals per 2.2