From 410e700d769d462c0601984e7fd123395cbc3865 Mon Sep 17 00:00:00 2001 From: DerThorsten Date: Mon, 12 Jun 2017 19:10:50 +0200 Subject: [PATCH 1/5] print __str__ instead of __repr__ in case of TypeError from incompatible function arguments. --- include/pybind11/pybind11.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 2bfd81cf38..3a549616b2 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -681,7 +681,14 @@ class cpp_function : public function { for (size_t ti = overloads->is_constructor ? 1 : 0; ti < args_.size(); ++ti) { if (!some_args) some_args = true; else msg += ", "; - msg += pybind11::repr(args_[ti]); + // using repr will lead to very + // long cout if a class implements + // __repr__ + // For array like classes this can even + // take a long time + // therefore we should use __str__ + //msg += pybind11::repr(args_[ti]); + msg += pybind11::str(args_[ti]); } if (kwargs_in) { auto kwargs = reinterpret_borrow(kwargs_in); From 8e1b46265454455ab4d64c3adfd06896b5959f81 Mon Sep 17 00:00:00 2001 From: DerThorsten Date: Mon, 12 Jun 2017 20:57:38 +0200 Subject: [PATCH 2/5] extendend options class to let users choose if __str__ or __repr__ should be used if argmuments mismatch --- include/pybind11/options.h | 10 ++++++++++ include/pybind11/pybind11.h | 14 ++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/include/pybind11/options.h b/include/pybind11/options.h index 3105551ddd..4a4c2eec7e 100644 --- a/include/pybind11/options.h +++ b/include/pybind11/options.h @@ -37,6 +37,11 @@ class options { options& disable_function_signatures() & { global_state().show_function_signatures = false; return *this; } options& enable_function_signatures() & { global_state().show_function_signatures = true; return *this; } + + options& enable_type_error_print_repr() { global_state().type_error_print_repr = true; return *this; } + options& enable_type_error_print_str() { global_state().type_error_print_repr = false; return *this; } + + // Getter methods (return the global state): @@ -44,6 +49,9 @@ class options { static bool show_function_signatures() { return global_state().show_function_signatures; } + static bool type_error_print_repr() { return global_state().type_error_print_repr; } + static bool type_error_print_str() { return !global_state().type_error_print_repr; } + // This type is not meant to be allocated on the heap. void* operator new(size_t) = delete; @@ -52,6 +60,8 @@ class options { struct state { bool show_user_defined_docstrings = true; //< Include user-supplied texts in docstrings. bool show_function_signatures = true; //< Include auto-generated function signatures in docstrings. + bool type_error_print_repr = true; //< If true, if args mismatch __repr__ of argument is shown if true, + //< else __str__ is used }; static state &global_state() { diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 3a549616b2..83d49ccfda 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -681,14 +681,12 @@ class cpp_function : public function { for (size_t ti = overloads->is_constructor ? 1 : 0; ti < args_.size(); ++ti) { if (!some_args) some_args = true; else msg += ", "; - // using repr will lead to very - // long cout if a class implements - // __repr__ - // For array like classes this can even - // take a long time - // therefore we should use __str__ - //msg += pybind11::repr(args_[ti]); - msg += pybind11::str(args_[ti]); + + if(options::type_error_print_repr()) + msg += pybind11::repr(args_[ti]); + else{ + msg += pybind11::str(args_[ti]); + } } if (kwargs_in) { auto kwargs = reinterpret_borrow(kwargs_in); From 25f4b4a439e48c91058e2e2815e62192a6e5fe3e Mon Sep 17 00:00:00 2001 From: DerThorsten Date: Mon, 12 Jun 2017 21:02:28 +0200 Subject: [PATCH 3/5] fixed wrong formating --- include/pybind11/options.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/options.h b/include/pybind11/options.h index 4a4c2eec7e..041aefea1a 100644 --- a/include/pybind11/options.h +++ b/include/pybind11/options.h @@ -37,7 +37,7 @@ class options { options& disable_function_signatures() & { global_state().show_function_signatures = false; return *this; } options& enable_function_signatures() & { global_state().show_function_signatures = true; return *this; } - + options& enable_type_error_print_repr() { global_state().type_error_print_repr = true; return *this; } options& enable_type_error_print_str() { global_state().type_error_print_repr = false; return *this; } @@ -60,7 +60,7 @@ class options { struct state { bool show_user_defined_docstrings = true; //< Include user-supplied texts in docstrings. bool show_function_signatures = true; //< Include auto-generated function signatures in docstrings. - bool type_error_print_repr = true; //< If true, if args mismatch __repr__ of argument is shown if true, + bool type_error_print_repr = true; //< If true, if args mismatch __repr__ of argument is shown if true, //< else __str__ is used }; From 20319dd474354bee9504123791aeedc0beecad21 Mon Sep 17 00:00:00 2001 From: DerThorsten Date: Mon, 12 Jun 2017 21:09:10 +0200 Subject: [PATCH 4/5] changed function names --- include/pybind11/options.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/options.h b/include/pybind11/options.h index 041aefea1a..bb0dd725f6 100644 --- a/include/pybind11/options.h +++ b/include/pybind11/options.h @@ -38,8 +38,8 @@ class options { options& enable_function_signatures() & { global_state().show_function_signatures = true; return *this; } - options& enable_type_error_print_repr() { global_state().type_error_print_repr = true; return *this; } - options& enable_type_error_print_str() { global_state().type_error_print_repr = false; return *this; } + options& set_type_error_print_repr() { global_state().type_error_print_repr = true; return *this; } + options& set_type_error_print_str() { global_state().type_error_print_repr = false; return *this; } From 1d45475851b5b52fccb49ee04b0567057d80655a Mon Sep 17 00:00:00 2001 From: DerThorsten Date: Tue, 13 Jun 2017 14:57:38 +0200 Subject: [PATCH 5/5] truncate repr in TypeError if repr is to long --- include/pybind11/options.h | 6 --- include/pybind11/pybind11.h | 15 ++++--- tests/CMakeLists.txt | 1 + tests/test_type_error_truncation.cpp | 45 +++++++++++++++++++ tests/test_type_error_truncation.py | 67 ++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 tests/test_type_error_truncation.cpp create mode 100644 tests/test_type_error_truncation.py diff --git a/include/pybind11/options.h b/include/pybind11/options.h index bb0dd725f6..2e2f1d8d78 100644 --- a/include/pybind11/options.h +++ b/include/pybind11/options.h @@ -38,10 +38,6 @@ class options { options& enable_function_signatures() & { global_state().show_function_signatures = true; return *this; } - options& set_type_error_print_repr() { global_state().type_error_print_repr = true; return *this; } - options& set_type_error_print_str() { global_state().type_error_print_repr = false; return *this; } - - // Getter methods (return the global state): @@ -49,8 +45,6 @@ class options { static bool show_function_signatures() { return global_state().show_function_signatures; } - static bool type_error_print_repr() { return global_state().type_error_print_repr; } - static bool type_error_print_str() { return !global_state().type_error_print_repr; } // This type is not meant to be allocated on the heap. void* operator new(size_t) = delete; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 83d49ccfda..a3e2667cae 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -681,12 +681,12 @@ class cpp_function : public function { for (size_t ti = overloads->is_constructor ? 1 : 0; ti < args_.size(); ++ti) { if (!some_args) some_args = true; else msg += ", "; - - if(options::type_error_print_repr()) - msg += pybind11::repr(args_[ti]); - else{ - msg += pybind11::str(args_[ti]); + const std::string argRepr = pybind11::repr(args_[ti]); + if(argRepr.size() > 100){ + msg += pybind11::str("{:.100} ...[truncated by pybind11]").format(argRepr); } + else + msg += argRepr; } if (kwargs_in) { auto kwargs = reinterpret_borrow(kwargs_in); @@ -697,7 +697,10 @@ class cpp_function : public function { for (auto kwarg : kwargs) { if (first) first = false; else msg += ", "; - msg += pybind11::str("{}={!r}").format(kwarg.first, kwarg.second); + const std::string argRepr = pybind11::repr( kwarg.second); + if(argRepr.size()>100){ + msg += pybind11::str("{}={!r:.100} ...[truncated by pybind11]").format(kwarg.first, argRepr); + } } } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d40c25ac4c..21ea9358bd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -56,6 +56,7 @@ set(PYBIND11_TEST_FILES test_smart_ptr.cpp test_stl_binders.cpp test_virtual_functions.cpp + test_type_error_truncation.cpp ) # Invoking cmake with something like: diff --git a/tests/test_type_error_truncation.cpp b/tests/test_type_error_truncation.cpp new file mode 100644 index 0000000000..0d23bbb1e1 --- /dev/null +++ b/tests/test_type_error_truncation.cpp @@ -0,0 +1,45 @@ +/* + tests/test_type_error_truncation.cpp -- exception translation + + Copyright (c) 2017 Thorsten Beier + + All rights reserved. Use of this source code is governed by a + BSD-style license that can be found in the LICENSE file. +*/ + +#include "pybind11_tests.h" +#include + + +// A type that should be raised as an exeption in Python +struct TypeWithLongRepr { + TypeWithLongRepr(const uint32_t reprSize = 100) + : reprSize_(reprSize){ + + } + std::string repr()const{ + std::string ret(reprSize_,'*'); + ret[0] = '<'; + ret[reprSize_-1] = '>'; + return ret; + } + void foo(const TypeWithLongRepr & , const std::string )const{ + + } + uint32_t reprSize_; +}; + + + +test_initializer type_error_truncation([](py::module &m) { + + py::class_(m, "TypeWithLongRepr") + .def(py::init()) + .def("__repr__", &TypeWithLongRepr::repr) + .def("foo",&TypeWithLongRepr::foo, + py::arg("arg1"), + py::arg("arg2") + ) + ; + +}); diff --git a/tests/test_type_error_truncation.py b/tests/test_type_error_truncation.py new file mode 100644 index 0000000000..b060bfc538 --- /dev/null +++ b/tests/test_type_error_truncation.py @@ -0,0 +1,67 @@ +import pytest +import pybind11_tests + + + + + + +def test_type_error_truncation(): + from pybind11_tests import TypeWithLongRepr + + size_of_repr = 200 + objA = TypeWithLongRepr(200) + objB = TypeWithLongRepr(50) + objC = TypeWithLongRepr(150) + reprStrA = repr(objA) + reprStrB = repr(objB) + reprStrC = repr(objC) + #print(reprStr) + assert len(reprStrA) == 200 + assert len(reprStrB) == 50 + assert len(reprStrC) == 150 + + + assert reprStrA.startswith('<') + assert reprStrA.endswith('>') + + + + with pytest.raises(TypeError) as excinfo: + objA.foo(objC, objC) + typeErrorMsg = str(excinfo.value) + print(typeErrorMsg) + assert len(typeErrorMsg) < 3 * 200 + assert typeErrorMsg.contains('...[truncated by pybind11]') + + + with pytest.raises(TypeError) as excinfo: + objB.foo(objB, objB) + typeErrorMsg = str(excinfo.value) + print(typeErrorMsg) + assert len(typeErrorMsg) < 3 * 200 + assert not typeErrorMsg.contains('...[truncated by pybind11]') + + + + + + with pytest.raises(TypeError) as excinfo: + objA.foo(objC, arg2=objC) + typeErrorMsg = str(excinfo.value) + print(typeErrorMsg) + assert len(typeErrorMsg) < 3 * 200 + assert typeErrorMsg.contains('...[truncated by pybind11]') + + + with pytest.raises(TypeError) as excinfo: + objB.foo(objB, objB) + typeErrorMsg = str(excinfo.value) + print(typeErrorMsg) + assert len(typeErrorMsg) < 3 * 200 + assert not typeErrorMsg.contains('...[truncated by pybind11]') + + + + +test_type_error_truncation() \ No newline at end of file