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

Numeric fixes #2176

Merged
merged 3 commits into from Nov 12, 2014
Merged

Numeric fixes #2176

merged 3 commits into from Nov 12, 2014

Conversation

cheald
Copy link
Contributor

@cheald cheald commented Nov 12, 2014

Un-exclude most tests in test_numeric and test_fixnum.

  • bitCoerce updated to use the new Numeric.doCoerce coercion
  • Numeric#step updated to accept kwargs and to enforce legacy step requirements.
  • Numeric#quo properly casts to rationals per 2.2

into fixnums or bignums, and raises an exception if it is unable to do so.
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.
@headius
Copy link
Member

headius commented Nov 12, 2014

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 // MRI: rb_some_function. You did that in a few places but not all.

Thanks! The Numeric#step logic looks like it gained a lot of missing functionality.

headius added a commit that referenced this pull request Nov 12, 2014
@headius headius merged commit 08059c3 into jruby:master Nov 12, 2014
@headius headius added the core label Nov 12, 2014
@headius headius added this to the JRuby 9.0.0.0-pre1 milestone Nov 12, 2014
@headius headius self-assigned this Nov 12, 2014
@cheald
Copy link
Contributor Author

cheald commented Nov 12, 2014

Will do, thanks. I rewrote that code probably twelve times yesterday, so it doesn't surprise me that I missed a few places. :)

@cheald cheald deleted the num_fixes branch November 12, 2014 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants