Fixing issue #1651 (errors when adding polygons with 4 points) - needs some testing

This commit is contained in:
Matthias Koefferlein 2024-03-23 15:24:18 +01:00
parent 230bacf725
commit 38a3b8305e
8 changed files with 54 additions and 31 deletions

View File

@ -350,9 +350,9 @@ match_method (int mid, PyObject *self, PyObject *args, PyObject *kwargs, bool st
PythonPtr arg (i >= argc ? get_kwarg (*a, kwargs) : (is_tuple ? PyTuple_GetItem (args, i) : PyList_GetItem (args, i)));
if (! arg) {
is_valid = a->spec ()->has_default ();
} else if (test_arg (*a, arg.get (), false /*strict*/)) {
} else if (test_arg (*a, arg.get (), false /*strict*/, false /*object substitution*/)) {
++sc;
} else if (test_arg (*a, arg.get (), true /*loose*/)) {
} else if (test_arg (*a, arg.get (), true /*loose*/, true /*object substitution*/)) {
// non-scoring match
} else {
is_valid = false;
@ -405,7 +405,7 @@ match_method (int mid, PyObject *self, PyObject *args, PyObject *kwargs, bool st
int i = 0;
for (gsi::MethodBase::argument_iterator a = meth->begin_arguments (); a != meth->end_arguments (); ++a, ++i) {
PythonPtr arg (i >= argc ? get_kwarg (*a, kwargs) : (is_tuple ? PyTuple_GetItem (args, i) : PyList_GetItem (args, i)));
if (arg && ! test_arg (*a, arg.get (), true /*loose*/)) {
if (arg && ! test_arg (*a, arg.get (), true /*loose*/, true /*object substitution*/)) {
return 0;
}
}
@ -1116,7 +1116,8 @@ property_setter_impl (int mid, PyObject *self, PyObject *value)
// check arguments (count and type)
bool is_valid = (*m)->compatible_with_num_args (1);
if (is_valid && ! test_arg (*(*m)->begin_arguments (), value, pass != 0 /*loose in the second pass*/)) {
bool loose = (pass != 0); // loose in the second pass
if (is_valid && ! test_arg (*(*m)->begin_arguments (), value, loose, loose)) {
is_valid = false;
}

View File

@ -1046,7 +1046,7 @@ size_t PythonBasedMapAdaptor::serial_size () const
template <class R>
struct test_arg_func
{
void operator() (bool *ret, PyObject *arg, const gsi::ArgType &atype, bool loose)
void operator() (bool *ret, PyObject *arg, const gsi::ArgType &atype, bool loose, bool /*object_substitution*/)
{
if ((atype.is_cptr () || atype.is_ptr ()) && arg == Py_None) {
@ -1083,7 +1083,7 @@ struct test_arg_func
template <>
struct test_arg_func<gsi::VariantType>
{
void operator() (bool *ret, PyObject *, const gsi::ArgType &, bool)
void operator() (bool *ret, PyObject *, const gsi::ArgType &, bool, bool)
{
// we assume we can convert everything into a variant
*ret = true;
@ -1093,7 +1093,7 @@ struct test_arg_func<gsi::VariantType>
template <>
struct test_arg_func<gsi::StringType>
{
void operator() (bool *ret, PyObject *arg, const gsi::ArgType &, bool)
void operator() (bool *ret, PyObject *arg, const gsi::ArgType &, bool, bool)
{
#if PY_MAJOR_VERSION < 3
if (PyString_Check (arg)) {
@ -1117,7 +1117,7 @@ struct test_arg_func<gsi::StringType>
template <>
struct test_arg_func<gsi::VectorType>
{
void operator() (bool *ret, PyObject *arg, const gsi::ArgType &atype, bool loose)
void operator() (bool *ret, PyObject *arg, const gsi::ArgType &atype, bool loose, bool /*object_substitution*/)
{
if ((atype.is_cptr () || atype.is_ptr ()) && arg == Py_None) {
// for ptr or cptr, null is an allowed value
@ -1138,7 +1138,7 @@ struct test_arg_func<gsi::VectorType>
size_t n = PyTuple_Size (arg);
for (size_t i = 0; i < n && *ret; ++i) {
if (! test_arg (ainner, PyTuple_GetItem (arg, i), loose)) {
if (! test_arg (ainner, PyTuple_GetItem (arg, i), loose, true /*issue-1651*/)) {
*ret = false;
}
}
@ -1147,7 +1147,7 @@ struct test_arg_func<gsi::VectorType>
size_t n = PyList_Size (arg);
for (size_t i = 0; i < n && *ret; ++i) {
if (! test_arg (ainner, PyList_GetItem (arg, i), loose)) {
if (! test_arg (ainner, PyList_GetItem (arg, i), loose, true /*issue-1651*/)) {
*ret = false;
}
}
@ -1159,7 +1159,7 @@ struct test_arg_func<gsi::VectorType>
template <>
struct test_arg_func<gsi::MapType>
{
void operator () (bool *ret, PyObject *arg, const gsi::ArgType &atype, bool loose)
void operator () (bool *ret, PyObject *arg, const gsi::ArgType &atype, bool loose, bool /*object_substitution*/)
{
if ((atype.is_cptr () || atype.is_ptr ()) && arg == Py_None) {
// for ptr or cptr, null is an allowed value
@ -1184,11 +1184,11 @@ struct test_arg_func<gsi::MapType>
PyObject *key, *value;
Py_ssize_t pos = 0;
while (PyDict_Next(arg, &pos, &key, &value)) {
if (! test_arg (ainner_k, key, loose)) {
if (! test_arg (ainner_k, key, loose, true /*issue-1651*/)) {
*ret = false;
break;
}
if (! test_arg (ainner, value, loose)) {
if (! test_arg (ainner, value, loose, true /*issue-1651*/)) {
*ret = false;
break;
}
@ -1199,7 +1199,7 @@ struct test_arg_func<gsi::MapType>
template <>
struct test_arg_func<gsi::ObjectType>
{
void operator() (bool *ret, PyObject *arg, const gsi::ArgType &atype, bool loose)
void operator() (bool *ret, PyObject *arg, const gsi::ArgType &atype, bool loose, bool object_substitution)
{
const gsi::ClassBase *acls = atype.cls ();
@ -1209,7 +1209,7 @@ struct test_arg_func<gsi::ObjectType>
return;
}
if (loose && (PyTuple_Check (arg) || PyList_Check (arg))) {
if (object_substitution && (PyTuple_Check (arg) || PyList_Check (arg))) {
// we may implicitly convert a tuple into a constructor call of a target object -
// for now we only check whether the number of arguments is compatible with the list given.
@ -1247,10 +1247,10 @@ struct test_arg_func<gsi::ObjectType>
};
bool
test_arg (const gsi::ArgType &atype, PyObject *arg, bool loose)
test_arg (const gsi::ArgType &atype, PyObject *arg, bool loose, bool object_substitution)
{
bool ret = false;
gsi::do_on_type<test_arg_func> () (atype.type (), &ret, arg, atype, loose);
gsi::do_on_type<test_arg_func> () (atype.type (), &ret, arg, atype, loose, object_substitution);
return ret;
}

View File

@ -69,7 +69,7 @@ PythonRef pull_arg (const gsi::ArgType &atype, gsi::SerialArgs &aserial, PYAObje
* @return True, if the type match
*/
bool
test_arg (const gsi::ArgType &atype, PyObject *arg, bool loose);
test_arg (const gsi::ArgType &atype, PyObject *arg, bool loose, bool object_substitution);
/**
* @brief Correct constness if a reference is const and a non-const reference is required

View File

@ -479,9 +479,9 @@ private:
VALUE arg = i >= argc ? get_kwarg (*a, kwargs) : argv[i];
if (arg == Qundef) {
is_valid = a->spec ()->has_default ();
} else if (test_arg (*a, arg, false /*strict*/)) {
} else if (test_arg (*a, arg, false /*strict*/, false /*with object substitution*/)) {
++sc;
} else if (test_arg (*a, arg, true /*loose*/)) {
} else if (test_arg (*a, arg, true /*loose*/, true /*with object substitution*/)) {
// non-scoring match
} else {
is_valid = false;

View File

@ -1053,7 +1053,7 @@ pull_arg (const gsi::ArgType &atype, Proxy *self, gsi::SerialArgs &aserial, tl::
template <class R>
struct test_arg_func
{
void operator () (bool *ret, VALUE arg, const gsi::ArgType &atype, bool loose)
void operator () (bool *ret, VALUE arg, const gsi::ArgType &atype, bool loose, bool /*object_substitution*/)
{
if ((atype.is_cptr () || atype.is_ptr ()) && arg == Qnil) {
@ -1101,7 +1101,7 @@ struct test_vector
unsigned int len = RARRAY_LEN(arr);
VALUE *el = RARRAY_PTR(arr);
while (len-- > 0) {
if (! test_arg (ainner, *el++, loose)) {
if (! test_arg (ainner, *el++, loose, true /*issue-1651*/)) {
*ret = false;
break;
}
@ -1112,7 +1112,7 @@ struct test_vector
template <>
struct test_arg_func<gsi::VectorType>
{
void operator () (bool *ret, VALUE arg, const gsi::ArgType &atype, bool loose)
void operator () (bool *ret, VALUE arg, const gsi::ArgType &atype, bool loose, bool /*object_substitution*/)
{
if ((atype.is_cptr () || atype.is_ptr ()) && arg == Qnil) {
// for pointers to vectors, nil is a valid value
@ -1141,11 +1141,11 @@ struct HashTestKeyValueData
static int hash_test_value_key (VALUE key, VALUE value, VALUE a)
{
HashTestKeyValueData *args = (HashTestKeyValueData *)a;
if (! test_arg (*args->ainner_k, key, args->loose)) {
if (! test_arg (*args->ainner_k, key, args->loose, true /*issue-1651*/)) {
*(args->ret) = false;
return ST_STOP;
}
if (! test_arg (*args->ainner, value, args->loose)) {
if (! test_arg (*args->ainner, value, args->loose, true /*issue-1651*/)) {
*(args->ret) = false;
return ST_STOP;
}
@ -1155,7 +1155,7 @@ static int hash_test_value_key (VALUE key, VALUE value, VALUE a)
template <>
struct test_arg_func<gsi::MapType>
{
void operator () (bool *ret, VALUE arg, const gsi::ArgType &atype, bool loose)
void operator () (bool *ret, VALUE arg, const gsi::ArgType &atype, bool loose, bool /*object_substitution*/)
{
if ((atype.is_cptr () || atype.is_ptr ()) && arg == Qnil) {
// for pointers to maps, nil is a valid value
@ -1183,14 +1183,14 @@ struct test_arg_func<gsi::MapType>
template <>
struct test_arg_func<gsi::ObjectType>
{
void operator () (bool *ret, VALUE arg, const gsi::ArgType &atype, bool loose)
void operator () (bool *ret, VALUE arg, const gsi::ArgType &atype, bool loose, bool object_substitution)
{
if ((atype.is_cptr () || atype.is_ptr ()) && arg == Qnil) {
// for const X * or X *, nil is an allowed value
*ret = true;
} else if (loose && TYPE (arg) == T_ARRAY) {
} else if (object_substitution && TYPE (arg) == T_ARRAY) {
// we may implicitly convert an array into a constructor call of a target object -
// for now we only check whether the number of arguments is compatible with the array given.
@ -1234,10 +1234,10 @@ struct test_arg_func<gsi::ObjectType>
};
bool
test_arg (const gsi::ArgType &atype, VALUE arg, bool loose)
test_arg (const gsi::ArgType &atype, VALUE arg, bool loose, bool object_substitution)
{
bool ret = false;
gsi::do_on_type<test_arg_func> () (atype.type (), &ret, arg, atype, loose);
gsi::do_on_type<test_arg_func> () (atype.type (), &ret, arg, atype, loose, object_substitution);
return ret;
}

View File

@ -62,7 +62,7 @@ void push_arg (const gsi::ArgType &atype, gsi::SerialArgs &aserial, VALUE arg, t
* otherwise:
* argument must be of the requested type
*/
bool test_arg (const gsi::ArgType &atype, VALUE arg, bool loose);
bool test_arg (const gsi::ArgType &atype, VALUE arg, bool loose, bool object_substitution);
}

View File

@ -746,6 +746,16 @@ class DBPolygonTests(unittest.TestCase):
poly = pya.Polygon(hull).insert_hole(hole1).insert_hole(hole2)
self.assertEqual(str(poly), "(0,0;0,3000;6000,3000;6000,0/1000,1000;2000,1000;2000,2000;1000,2000/3000,1000;4000,1000;4000,2000;3000,2000)")
def test_argumentShortcuts(self):
# implicit conversion to a Point array:
poly = pya.Polygon([ (0,0), (0,1000), (1000,1000) ])
self.assertEqual(str(poly), "(0,0;0,1000;1000,1000)")
# issue 1651 - no binding to Box constructor
poly = pya.Polygon([ (0,0), (0,1000), (1000,1000), (1000,0) ])
self.assertEqual(str(poly), "(0,0;0,1000;1000,1000;1000,0)")
# run unit tests
if __name__ == '__main__':
suite = unittest.TestLoader().loadTestsFromTestCase(DBPolygonTests)

View File

@ -834,6 +834,18 @@ class DBPolygon_TestClass < TestBase
end
def test_argumentShortcuts
# implicit conversion to a Point array:
poly = RBA::Polygon.new([ [0,0], [0,1000], [1000,1000] ])
assert_equal(poly.to_s, "(0,0;0,1000;1000,1000)")
# issue 1651 - no binding to Box constructor
poly = RBA::Polygon.new([ [0,0], [0,1000], [1000,1000], [1000,0] ])
assert_equal(poly.to_s, "(0,0;0,1000;1000,1000;1000,0)")
end
end
load("test_epilogue.rb")