This should go into 3.0.0
Did you remove therelease 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 convertingnp.float64 toSystem.Double when the requested type isSystem.Double which was explicitly addressed in your PR. It is about what to convert to when the requested type isSystem.Object.

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

Convertingnp.float64 toSystem.Double withoutDouble 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 decodefloat 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 exceptnumpy.float64 becausenumpy.float64 derives fromfloat but other NumPy types do not, sofloat64 will be converted toSystem.Double, which does not have any NumPy members on it.

[ForbidPythonThreads]dynamicCentroid(dynamicvalue)=>value-value.mean();

v=numpy.float32(42)print(Centroid(v))# OKv=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 toObject. 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 toint for much the same reason, and integer objects in this situation are converted toPyInt precisely becauseint does not map directly to any ofSystem.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 toint for much the same reason, and integer objects in this situation are converted toPyInt precisely becauseint does not map directly to any ofSystem.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 imaginaryDictionary<object, object>.GetOrDefault)
2. Functions that use Python members and thus want to use aPyObject

For the latter, I don't see any reason why these shouldn't just take aPyObject parameter. I get thatdynamic makes things more convenient, but adynamic 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 apythonnet.conversion.disable(method_wrapper) call (or a correspondingPython.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'sfloat64 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 aSystem.Object parameter in library code is that you either only ever plan to call.ToString and/or.GetHashCode, or your parameter is actually adynamic. And since the first scenario is not broken after we added GIL to these members, the only remaining scenario for aSystem.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'sfloat64 though has over 40 members on top of that. And they add new ones now and then.

That is not the point.float hasis_integer,conjugate, etc. If you want to access those viadynamic, it would not work either. Same for all of thestr 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 aSystem.Object parameter in library code is that you either only ever plan to call.ToString and/or.GetHashCode, or your parameter is actually adynamic. And since the first scenario is not broken after we added GIL to these members, the only remaining scenario for aSystem.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 distinguishdynamic fromobject 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 wherefloat should be passed toSystem.Object parameter and converted toSystem.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 wherefloat should be passed toSystem.Object parameter and converted toSystem.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 forobject 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.