Implicit float conversion in function calls

[filmor]

What does this implement/fix? Explain your changes.

Uses __float__ when available if a Python object is passed to a function with a Double or Single parameter.

Does this close any currently open issues?

#1833

Any other comments?

This is not yet supported for integer types, which would use __int__
or __index__ (for Python >=3.10), as the respective logic is for some
reason only implemented for AsLongLong and AsLong, not for the
unsigned counterparts and also not for the AsSize_t variants that we
are using.

Checklist

Check all those that are applicable and complete.

• Make sure to include one or more tests for your change
• If an enhancement PR, please create docs and at best an example
• Ensure you have signed the .NET Foundation CLA
• Add yourself to AUTHORS
• Updated the CHANGELOG

[lostmsu]

Implicit casts to float were disabled intentionally. For instance, float(torch.tensor(42)) is a float. However, if implicit casts are enabled, Python.NET can not choose an overload between float and Tensor if the C# side has a Tensor wrapper via a custom codec.

[filmor]

The (expected) order here is:

1. Check if a type matches exactly
2. Check if a custom codec matches
3. Check if the parameter type is Double or Single and if that is the case and the Python parameter is "floaty" (i.e. PyNumber_Float returns something != NULL), use the converted value
4. Check if the parameter type is an integer type and do the same with PyNumber_Long or PyNumber_Index

This should lead to a sensible overload resolution.

[lostmsu]

Check if a type matches exactly

This can never happen with a custom Python type.

Also, we do not really perform overload resolution prioritization like that. E.g. the __call__ impl just passes the object to Convert and checks if conversion succeeded.

[filmor]

Can you please provide a test-case that you expect to fail with this PR? I'm fine with working on the overload resolution, but I think making CsharpFunc(float_like) fail is complicating things on the user side in a very error-prone manner.

[lostmsu]

I described the test case above.

1. Create an empty class Tensor in C#, and a codec along the lines of return isinstance(o, torch.tensor) ? new Tensor() : null;
2. Get a C# function with two overloads: Foo(float) (or maybe Foo(double)) and Foo(Tensor)
3. Call it from Python with Foo(torch.tensor(42))

See it pick the overload based on the weather. This is because we actually do not perform any overload resolution, but this behavior prevents user from forcing exact type match with codecs when the type on Python side has __float__.

If somebody wants this behavior, they can register a codec for types implementing __float__ or just explicitly cast their floaty types to float.

What we might want is for System.Double(np_float64_val) to produce a Double value (which I am not sure it does currently)`.

[lostmsu]

@filmor I think we still should not just enable __float__ for all types that have it, because there are bogus ones that do that and should never be converted without an explicit cast. Implicit conversion should be reserved only for types, that explicitly represent identically sized numbers, e.g. numpy.float64, ctypes.int32, etc

[filmor]

That effectively boils down to just using the "real" Runtime.PyFloat_Check instead of our botched version. I'd be completely fine with that.

[filmor]

Just to recap: This will now check before attempting a conversion whether the object identifies as a float using PyFloat_Check, which has been adjusted in the way @rzindler proposed in #1936. All other changes are just cosmetics (plus one minor bug-fix adjustment for the Int64 conversion) and test-cases.

[filmor]

@lostmsu If this is mergeable for you, I think that would solve the reported issue sufficiently that we can continue with the releases without having to touch method resolution right now (which I also really don't want to). PyTorch's tensor does not derive from float64, so this particular case should not be a problem.

[lostmsu]

My problem with this approach is that it does not allow us to do:

string Foo(float x) => "float32";
string Foo(double x) => "float64";

Foo(numpy.float32(42.0))

[filmor]

My problem with this approach is that it does not allow us to do:

string Foo(float x) => "float32";
string Foo(double x) => "float64";

Foo(numpy.float32(42.0))

It does, you just need to be explicit. But I get your point, I just think that the reasonable default is more important than optimising this overload. We can document this particular issue.

[rzindler]

Thanks guys!

[lostmsu]

I think we can do better than document, and actually special case ctypes, NumPy, PyTorch, and TensorFlow types that have exact precision. Preferably also leave a room for users to special case other precise types.

It is not that hard, and if we don't do it now we'd have to wait until 4.0 to make another breaking change.

[filmor]

I'm strongly against special-casing particular library types. If you want to allow proper float32, that's an entirely new issue. Allowing modifying primitive type conversion as well.

[lostmsu]

That's exactly what I want to do in a week or two.

[filmor]

That's exactly what I want to do in a week or two.

I'll merge this in the meantime. Feel free to revert or adjust the behaviour when you are attempting your refactoring. In this state, I'd be fine to publish it.