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

[ji] (auto) convert java numbers #4718

Closed
wants to merge 11 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@kares
Member

kares commented Jul 17, 2017

Java Integration does not fully handle Java's Number type system (primitives as well as java.math classes).

mostly Java numeric values are converted back and forth as needed, however there's still a fay to explicitly instanciate Java types (java.lang.Integer.new(10) etc).

improvements presented here try to fix those Java value proxies to seamlessly work with Ruby's conversion rules (e.g. to_int). there has been requests on these already, added test demonstrate what's now possible.

@kares

This comment has been minimized.

Member

kares commented Jul 17, 2017

@headius am happy to move off and merge this to ruby-2.4 (expect some conflicts with your recent work)
continued the experiments against master since wanted a fully from test and CI seems to still fail for 2.4

@headius

Looks good to me. It will be nice to have these types behave well in Ruby-land. Few minor items to fix or discuss.

throw runtime.newTypeError(obj, "Float");
}
return (RubyFloat) TypeConverter.convertToType(obj, runtime.getFloat(), "to_f", true);
}

This comment has been minimized.

@headius

headius Jul 18, 2017

Member

The old TypeConverter.toFloat appears to only have been used by Pack. Perhaps we can just replace it with the newer impl rather than having two around? I see in MRI it is used in pack.c, but also in random.c. rb_to_float itself is in object.c.

This comment has been minimized.

@kares

kares Jul 18, 2017

Member

OK thanks, will look into it ...

// public IRubyObject fdiv(ThreadContext context, IRubyObject other) {
// return f_div(context, f_to_f(context, this), other);
// }

This comment has been minimized.

@headius

headius Jul 18, 2017

Member

I fixed some of these names on the 2.4 branch, so there may be some minor conflicts. No need to keep this commented code in any case.

final IRubyObject value;
if (val instanceof java.math.BigDecimal) {
final RubyClass klass = context.runtime.getClass("BigDecimal");
if (klass == null) { // user should require 'bigdecimal'

This comment has been minimized.

@headius

headius Jul 18, 2017

Member

Perhaps we should autoload? I was just thinking yesterday it's odd that BigDecimal is the last numeric type not always loaded. Maybe we should point that out to ruby-core as well?

This comment has been minimized.

@kares

kares Jul 18, 2017

Member

guess its not that common to use - like in Java, BigInteger is probably used more than BigDecimal.
... but you're right if all others are core maybe this shouldn't be a lib.
was also thinking of BigDecimal auto-converting Java<->Ruby, however more tricky when the library isn't loaded.

This comment has been minimized.

@headius

headius Jul 18, 2017

Member

It seems like many more people would use BigDecimal than Complex to me...I'll ping MRI folks about it.

@kares

This comment has been minimized.

Member

kares commented Jul 19, 2017

should now be 💚 (based on feedback) ... will pbly wait for the ruby-2.4 merge to resolve potential conflicts

@kares kares added this to the JRuby 9.2.0.0 milestone Jul 19, 2017

@headius

This comment has been minimized.

Member

headius commented Aug 4, 2017

The 2.4 merge will happen tomorrow (Friday 4 Aug).

@kares

This comment has been minimized.

Member

kares commented Aug 8, 2017

merged onto master manually: cd08091...2a5b495

@kares kares closed this Aug 8, 2017

@kares kares deleted the convert-java-numbers branch Aug 8, 2017

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