This should go into 3.0.0
Did you remove the release branch?

3.0.0 ia released already and I disagree with the proposed behavior. I don't get why you are insisting that this is the "correct" way. If a type is derived from another that clearly indicates an "is a" relation, and evidently people got really confused with the behavior we had in the RCs. I totally agree that we should allow overriding this behavior, I disagree with this approach.

@filmor I don't think you got the scenario. It is not about converting np.float64 to System.Double when the requested type is System.Double which was explicitly addressed in your PR. It is about what to convert to when the requested type is System.Object.

Without this change codecs can't be applied because the conversion to primitives takes priority.

Converting np.float64 to System.Double without Double being the desired type explicitly is also a loss of information, because at the receiving side you can no longer know that the original value was more specific.

@filmor any comments? Due to the above type information loss either this change needs to be accepted to be able to avoid the issue, or the checks need to move below codec application, which can lead to poorly predictable behavior for codecs that claim to be able to decode float itself.

I'm on vacation right now, will check once I'm back. I find it unlikely that I will accept the PR as it stands, but I'll need to be a bit around with it.

@filmor I think I came up with a good demonstration why this PR should be accepted as is. Without it the following code will work for any numpy value except numpy.float64 because numpy.float64 derives from float but other NumPy types do not, so float64 will be converted to System.Double, which does not have any NumPy members on it.

[ForbidPythonThreads]
dynamic Centroid(dynamic value)  => value - value.mean();

v = numpy.float32(42)
print(Centroid(v)) # OK
v = numpy.float64(42)
# RuntimeBinderException: System.Double does not have .mean()
print(Centroid(v))

Codecs are not involved here, so the conversion should not happen regardless of the state of the codecs.

With that argument, we should forbid /any/ conversion of primitive Python types to Object. Maybe this case should just be handled as an attribute?

@filmor I don't think it applies to primitive types, as

1. they don't really have that many members specific to Python
2. they are present in all Python installations and have natural mapping to .NET types
3. we actually already did that to int for much the same reason, and integer objects in this situation are converted to PyInt precisely because int does not map directly to any of System.Int*

I don't think it applies to primitive types, as

1. they don't really have that many members specific to Python

I'm not sure what you mean by that. How many would be "too many"?

3. we actually already did that to int for much the same reason, and integer objects in this situation are converted to PyInt precisely because int does not map directly to any of System.Int*

Exactly. And I think instead of breaking working code further, we should just make this an option. There are (to me) two kinds of functions for which this is relevant:

1. Functions that pass an element through (e.g. an imaginary Dictionary<object, object>.GetOrDefault)
2. Functions that use Python members and thus want to use a PyObject

For the latter, I don't see any reason why these shouldn't just take a PyObject parameter. I get that dynamic makes things more convenient, but a dynamic Centroid(PyObject obj) { dynamic o = obj; return obj - obj.mean(); } is not a lot more work. For these, an additional attribute would also do nicely, e.g. [DisablePythonnetConversions] or [PythonnetFunction(conversion = None, gil = true)].

For the former, things might be a bit more tricky, but I'd suggest (in general) to just expose more of the conversion machinery publicly and solve this case using a pythonnet.conversion.disable(method_wrapper) call (or a corresponding Python.Runtime call in C#).

I'm not sure what you mean by that. How many would be "too many"?

float is a primitive type and only has primitive operations implemented. Addition, subtraction, etc. There are maybe a few 10 ops in total, and the set hasn't changed for 10+ years. NumPy's float64 though has over 40 members on top of that. And they add new ones now and then.

For the latter, I don't see any reason why these shouldn't just take a PyObject parameter.

Then you proceed to name a reason. I'd also say that the only good reason to have a System.Object parameter in library code is that you either only ever plan to call .ToString and/or .GetHashCode, or your parameter is actually a dynamic. And since the first scenario is not broken after we added GIL to these members, the only remaining scenario for a System.Object parameter is made worse by this autoconversion.

I'm not sure what you mean by that. How many would be "too many"?

float is a primitive type and only has primitive operations implemented. Addition, subtraction, etc. There are maybe a few 10 ops in total, and the set hasn't changed for 10+ years. NumPy's float64 though has over 40 members on top of that. And they add new ones now and then.

That is not the point. float has is_integer, conjugate, etc. If you want to access those via dynamic, it would not work either. Same for all of the str members.

For the latter, I don't see any reason why these shouldn't just take a PyObject parameter.

Then you proceed to name a reason. I'd also say that the only good reason to have a System.Object parameter in library code is that you either only ever plan to call .ToString and/or .GetHashCode, or your parameter is actually a dynamic. And since the first scenario is not broken after we added GIL to these members, the only remaining scenario for a System.Object parameter is made worse by this autoconversion.

And I also give a way to make this work by being more explicit. That is the gist of a lot of the changes for Python.NET 3 (that you also introduced yourself!). There is no way to distinguish dynamic from object parameters, so the proper way to modify this behaviour without breaking existing code is to introduce attributes or runtime configuration.

And I also give a way to make this work by being more explicit.

Point is why even do it this way in the first place? What is the scenario where float should be passed to System.Object parameter and converted to System.Double?

And I also give a way to make this work by being more explicit.

Point is why even do it this way in the first place? What is the scenario where float should be passed to System.Object parameter and converted to System.Double?

We agree here. I just want to not break the behaviour right now, and if we break it, remove all special-casing in one go instead of keeping it for (some) primitives.

If we want to keep things simple, we can make this a runtime configuration parameter.

@filmor as far as I am concerned, this issue is a consequence of introducing last minute changes to 3.0 before release. And considering its use case rarity, it should be considered a bug in 3.0.0, so should be fixed in 3.0.1. If you are unhappy about interface breakage, 3.0.0 should be yanked.

It is not. As said, with the change as it stands, I'm not okay and I think we have discussed it enough now. The correct thing to do would be to not convert at all for object parameters, which is a much bigger breakage (compared to 2.5) and can't be introduced in a patch version. I'll draft a PR to enforce this behaviour with runtime configuration.

@filmor I don't think we need a global toggle for this. There was another option:

the checks need to move below codec application

That way this could be fixed with a codec.

Closing this in favour of #1986.