Skip to content

Disabled float and bool implicit conversions #1584

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

Merged
merged 1 commit into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ See [Mixins/collections.py](src/runtime/Mixins/collections.py).
- BREAKING: When trying to convert Python `int` to `System.Object`, result will
be of type `PyInt` instead of `System.Int32` due to possible loss of information.
Python `float` will continue to be converted to `System.Double`.
- BREAKING: Python.NET will no longer implicitly convert types like `numpy.float64`, that implement `__float__` to
`System.Single` and `System.Double`. An explicit conversion is required on Python or .NET side.
- BREAKING: Python.NET will no longer implicitly convert any Python object to `System.Boolean`.
- BREAKING: `PyObject.GetAttr(name, default)` now only ignores `AttributeError` (previously ignored all exceptions).
- BREAKING: `PyObject` no longer implements `IEnumerable<PyObject>`.
Instead, `PyIterable` does that.
Expand Down
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Example
dynamic sin = np.sin;
Console.WriteLine(sin(5));

double c = np.cos(5) + sin(5);
double c = (double)(np.cos(5) + sin(5));
Console.WriteLine(c);

dynamic a = np.array(new List<float> { 1, 2, 3 });
Expand Down
2 changes: 1 addition & 1 deletion src/embed_tests/NumPyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void TestReadme()
dynamic sin = np.sin;
StringAssert.StartsWith("-0.95892", sin(5).ToString());

double c = np.cos(5) + sin(5);
double c = (double)(np.cos(5) + sin(5));
Assert.AreEqual(-0.675262, c, 0.01);

dynamic a = np.array(new List<float> { 1, 2, 3 });
Expand Down
7 changes: 7 additions & 0 deletions src/embed_tests/TestConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ public void ConvertOverflow()
}
}

[Test]
public void NoImplicitConversionToBool()
{
var pyObj = new PyList(items: new[] { 1.ToPython(), 2.ToPython() }).ToPython();
Assert.Throws<InvalidCastException>(() => pyObj.As<bool>());
}

[Test]
public void ToNullable()
{
Expand Down
4 changes: 2 additions & 2 deletions src/python_tests_runner/PythonTestRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ public void Dispose()
static IEnumerable<string[]> PythonTestCases()
{
// Add the test that you want to debug here.
yield return new[] { "test_enum", "test_enum_standard_attrs" };
yield return new[] { "test_generic", "test_missing_generic_type" };
yield return new[] { "test_indexer", "test_boolean_indexer" };
yield return new[] { "test_delegate", "test_bool_delegate" };
}

/// <summary>
Expand Down
73 changes: 71 additions & 2 deletions src/runtime/converter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Reflection;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -501,6 +502,44 @@ internal static bool ToManagedValue(IntPtr value, Type obType,
return ToPrimitive(value, obType, out result, setError);
}

/// <remarks>
/// Unlike <see cref="ToManaged(BorrowedReference, Type, out object?, bool)"/>,
/// this method does not have a <c>setError</c> parameter, because it should
/// only be called after <see cref="ToManaged(BorrowedReference, Type, out object?, bool)"/>.
/// </remarks>
internal static bool ToManagedExplicit(BorrowedReference value, Type obType,
out object? result)
{
result = null;

// this method would potentially clean any existing error resulting in information loss
Debug.Assert(Runtime.PyErr_Occurred() == null);

string? converterName =
IsInteger(obType) ? "__int__"
: IsFloatingNumber(obType) ? "__float__"
: null;

if (converterName is null) return false;

Debug.Assert(obType.IsPrimitive);

using var converter = Runtime.PyObject_GetAttrString(value, converterName);
if (converter.IsNull())
{
Exceptions.Clear();
return false;
}

using var explicitlyCoerced = Runtime.PyObject_CallObject(converter, BorrowedReference.Null);
if (explicitlyCoerced.IsNull())
{
Exceptions.Clear();
return false;
}
return ToPrimitive(explicitlyCoerced, obType, out result, false);
}

static object? ToPyObjectSubclass(ConstructorInfo ctor, PyObject instance, bool setError)
{
try
Expand Down Expand Up @@ -544,6 +583,8 @@ internal static int ToInt32(BorrowedReference value)
return checked((int)num);
}

private static bool ToPrimitive(BorrowedReference value, Type obType, out object? result, bool setError)
=> ToPrimitive(value.DangerousGetAddress(), obType, out result, setError);
/// <summary>
/// Convert a Python value to an instance of a primitive managed type.
/// </summary>
Expand Down Expand Up @@ -590,8 +631,21 @@ private static bool ToPrimitive(IntPtr value, Type obType, out object? result, b
}

case TypeCode.Boolean:
result = Runtime.PyObject_IsTrue(value) != 0;
return true;
if (value == Runtime.PyTrue)
{
result = true;
return true;
}
if (value == Runtime.PyFalse)
{
result = false;
return true;
}
if (setError)
{
goto type_error;
}
return false;

case TypeCode.Byte:
{
Expand Down Expand Up @@ -768,6 +822,10 @@ private static bool ToPrimitive(IntPtr value, Type obType, out object? result, b

case TypeCode.Single:
{
if (!Runtime.PyFloat_Check(value) && !Runtime.PyInt_Check(value))
{
goto type_error;
}
double num = Runtime.PyFloat_AsDouble(value);
if (num == -1.0 && Exceptions.ErrorOccurred())
{
Expand All @@ -786,6 +844,10 @@ private static bool ToPrimitive(IntPtr value, Type obType, out object? result, b

case TypeCode.Double:
{
if (!Runtime.PyFloat_Check(value) && !Runtime.PyInt_Check(value))
{
goto type_error;
}
double num = Runtime.PyFloat_AsDouble(value);
if (num == -1.0 && Exceptions.ErrorOccurred())
{
Expand Down Expand Up @@ -933,6 +995,13 @@ private static bool ToArray(IntPtr value, Type obType, out object? result, bool
result = items;
return true;
}

internal static bool IsFloatingNumber(Type type) => type == typeof(float) || type == typeof(double);
internal static bool IsInteger(Type type)
=> type == typeof(Byte) || type == typeof(SByte)
|| type == typeof(Int16) || type == typeof(UInt16)
|| type == typeof(Int32) || type == typeof(UInt32)
|| type == typeof(Int64) || type == typeof(UInt64);
}

public static class ConverterExtension
Expand Down
16 changes: 15 additions & 1 deletion src/runtime/pyobject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1293,7 +1293,21 @@ public override bool TryInvoke(InvokeBinder binder, object[] args, out object re

public override bool TryConvert(ConvertBinder binder, out object result)
{
return Converter.ToManaged(this.obj, binder.Type, out result, false);
// always try implicit conversion first
if (Converter.ToManaged(this.obj, binder.Type, out result, false))
{
return true;
}

if (binder.Explicit)
{
Runtime.PyErr_Fetch(out var errType, out var errValue, out var tb);
bool converted = Converter.ToManagedExplicit(Reference, binder.Type, out result);
Runtime.PyErr_Restore(errType.StealNullable(), errValue.StealNullable(), tb.StealNullable());
return converted;
}

return false;
}

public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg, out object result)
Expand Down
47 changes: 15 additions & 32 deletions tests/test_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,53 +13,36 @@ def test_bool_conversion():
"""Test bool conversion."""
ob = ConversionTest()
assert ob.BooleanField is False
assert ob.BooleanField is False
assert ob.BooleanField == 0

ob.BooleanField = True
assert ob.BooleanField is True
assert ob.BooleanField is True
assert ob.BooleanField == 1

ob.BooleanField = False
assert ob.BooleanField is False
assert ob.BooleanField is False
assert ob.BooleanField == 0

ob.BooleanField = 1
assert ob.BooleanField is True
assert ob.BooleanField is True
assert ob.BooleanField == 1

ob.BooleanField = 0
assert ob.BooleanField is False
assert ob.BooleanField is False
assert ob.BooleanField == 0
with pytest.raises(TypeError):
ob.BooleanField = 1

with pytest.raises(TypeError):
ob.BooleanField = 0

ob.BooleanField = System.Boolean(None)
assert ob.BooleanField is False
assert ob.BooleanField is False
assert ob.BooleanField == 0
with pytest.raises(TypeError):
ob.BooleanField = None

ob.BooleanField = System.Boolean('')
assert ob.BooleanField is False
assert ob.BooleanField is False
assert ob.BooleanField == 0
with pytest.raises(TypeError):
ob.BooleanField = ''

ob.BooleanField = System.Boolean(0)
assert ob.BooleanField is False
assert ob.BooleanField is False
assert ob.BooleanField == 0
with pytest.raises(TypeError):
ob.BooleanField = System.Boolean(0)

ob.BooleanField = System.Boolean(1)
assert ob.BooleanField is True
assert ob.BooleanField is True
assert ob.BooleanField == 1
with pytest.raises(TypeError):
ob.BooleanField = System.Boolean(1)

ob.BooleanField = System.Boolean('a')
assert ob.BooleanField is True
assert ob.BooleanField is True
assert ob.BooleanField == 1
with pytest.raises(TypeError):
ob.BooleanField = System.Boolean('a')


def test_sbyte_conversion():
Expand Down
8 changes: 7 additions & 1 deletion tests/test_delegate.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ def test_bool_delegate():
from Python.Test import BoolDelegate

def always_so_negative():
return 0
return False

d = BoolDelegate(always_so_negative)
ob = DelegateTest()
Expand All @@ -256,6 +256,12 @@ def always_so_negative():
assert not d()
assert not ob.CallBoolDelegate(d)

def always_so_positive():
return 1
bad = BoolDelegate(always_so_positive)
with pytest.raises(TypeError):
ob.CallBoolDelegate(bad)

def test_object_delegate():
"""Test object delegate."""
from Python.Test import ObjectDelegate
Expand Down
8 changes: 4 additions & 4 deletions tests/test_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,11 @@ def test_boolean_field():
ob.BooleanField = False
assert ob.BooleanField is False

ob.BooleanField = 1
assert ob.BooleanField is True
with pytest.raises(TypeError):
ob.BooleanField = 1

ob.BooleanField = 0
assert ob.BooleanField is False
with pytest.raises(TypeError):
ob.BooleanField = 0


def test_sbyte_field():
Expand Down
14 changes: 8 additions & 6 deletions tests/test_indexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,15 @@ def test_boolean_indexer():
ob = Test.BooleanIndexerTest()

assert ob[True] is None
assert ob[1] is None

ob[0] = "false"
assert ob[0] == "false"

ob[1] = "true"
assert ob[1] == "true"
with pytest.raises(TypeError):
ob[1]
with pytest.raises(TypeError):
ob[0]
with pytest.raises(TypeError):
ob[1] = "true"
with pytest.raises(TypeError):
ob[0] = "false"

ob[False] = "false"
assert ob[False] == "false"
Expand Down
4 changes: 0 additions & 4 deletions tests/test_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ def test_preload_var():
try:
clr.setPreload(True)
assert clr.getPreload() is True, clr.getPreload()
clr.setPreload(0)
assert clr.getPreload() is False, clr.getPreload()
clr.setPreload(1)
assert clr.getPreload() is True, clr.getPreload()

import System.Configuration
content = dir(System.Configuration)
Expand Down