Skip to content

Commit

Permalink
Remove newlines from docstring signature (google#4735)
Browse files Browse the repository at this point in the history
* Remove newlines from docstring signature

* Jean/dev (google#1)

Replace newlines in arg values with spaces

* style: pre-commit fixes

* Don't use std::find_if for C++ 11 compatibility

* Avoid implicit char to bool conversion

* Test default arguments for line breaks

* style: pre-commit fixes

* Separate Eigen tests

* style: pre-commit fixes

* Fix merge

* Try importing numpy

* Avoid unreferenced variable in catch block

* style: pre-commit fixes

* Update squash function

* Reduce try block

* Additional test cases

* style: pre-commit fixes

* Put statement inside braces

* Move string into function body

* Rename repr for better readability. Make constr explicit.

* Add multiline string default argument test case

* style: pre-commit fixes

* Add std namespace, do not modify string repr

* Test for all space chars, test str repr not modified

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
JeanElsner and pre-commit-ci[bot] authored Aug 15, 2023
1 parent f47ff32 commit b9359ce
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 1 deletion.
41 changes: 40 additions & 1 deletion include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,45 @@ PYBIND11_WARNING_DISABLE_MSVC(4127)

PYBIND11_NAMESPACE_BEGIN(detail)

inline std::string replace_newlines_and_squash(const char *text) {
const char *whitespaces = " \t\n\r\f\v";
std::string result(text);
bool previous_is_whitespace = false;

// Do not modify string representations
char first_char = result[0];
char last_char = result[result.size() - 1];
if (first_char == last_char && first_char == '\'') {
return result;
}
result.clear();

// Replace characters in whitespaces array with spaces and squash consecutive spaces
while (*text != '\0') {
if (std::strchr(whitespaces, *text)) {
if (!previous_is_whitespace) {
result += ' ';
previous_is_whitespace = true;
}
} else {
result += *text;
previous_is_whitespace = false;
}
++text;
}

// Strip leading and trailing whitespaces
const size_t str_begin = result.find_first_not_of(whitespaces);
if (str_begin == std::string::npos) {
return "";
}

const size_t str_end = result.find_last_not_of(whitespaces);
const size_t str_range = str_end - str_begin + 1;

return result.substr(str_begin, str_range);
}

// Apply all the extensions translators from a list
// Return true if one of the translators completed without raising an exception
// itself. Return of false indicates that if there are other translators
Expand Down Expand Up @@ -424,7 +463,7 @@ class cpp_function : public function {
// Write default value if available.
if (!is_starred && arg_index < rec->args.size() && rec->args[arg_index].descr) {
signature += " = ";
signature += rec->args[arg_index].descr;
signature += detail::replace_newlines_and_squash(rec->args[arg_index].descr);
}
// Separator for positional-only arguments (placed after the
// argument, rather than before like *
Expand Down
17 changes: 17 additions & 0 deletions tests/test_eigen_matrix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,23 @@ TEST_SUBMODULE(eigen_matrix, m) {
m.def("dense_c", [mat]() -> DenseMatrixC { return DenseMatrixC(mat); });
m.def("dense_copy_r", [](const DenseMatrixR &m) -> DenseMatrixR { return m; });
m.def("dense_copy_c", [](const DenseMatrixC &m) -> DenseMatrixC { return m; });
// test_defaults
bool have_numpy = true;
try {
py::module_::import("numpy");
} catch (const py::error_already_set &) {
have_numpy = false;
}
if (have_numpy) {
py::module_::import("numpy");
Eigen::Matrix<double, 3, 3> defaultMatrix = Eigen::Matrix3d::Identity();
m.def(
"defaults_mat", [](const Eigen::Matrix3d &) {}, py::arg("mat") = defaultMatrix);

Eigen::VectorXd defaultVector = Eigen::VectorXd::Ones(32);
m.def(
"defaults_vec", [](const Eigen::VectorXd &) {}, py::arg("vec") = defaultMatrix);
}
// test_sparse, test_sparse_signature
m.def("sparse_r", [mat]() -> SparseMatrixR {
// NOLINTNEXTLINE(clang-analyzer-core.uninitialized.UndefReturn)
Expand Down
5 changes: 5 additions & 0 deletions tests/test_eigen_matrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,11 @@ def test_dense_signature(doc):
)


def test_defaults(doc):
assert "\n" not in str(doc(m.defaults_mat))
assert "\n" not in str(doc(m.defaults_vec))


def test_named_arguments():
a = np.array([[1.0, 2], [3, 4], [5, 6]])
b = np.ones((2, 1))
Expand Down
44 changes: 44 additions & 0 deletions tests/test_kwargs_and_defaults.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,50 @@ TEST_SUBMODULE(kwargs_and_defaults, m) {
m.def("kw_func_udl", kw_func, "x"_a, "y"_a = 300);
m.def("kw_func_udl_z", kw_func, "x"_a, "y"_a = 0);

// test line breaks in default argument representation
struct CustomRepr {
std::string repr_string;

explicit CustomRepr(const std::string &repr) : repr_string(repr) {}

std::string __repr__() const { return repr_string; }
};

py::class_<CustomRepr>(m, "CustomRepr")
.def(py::init<const std::string &>())
.def("__repr__", &CustomRepr::__repr__);

m.def(
"kw_lb_func0",
[](const CustomRepr &) {},
py::arg("custom") = CustomRepr(" array([[A, B], [C, D]]) "));
m.def(
"kw_lb_func1",
[](const CustomRepr &) {},
py::arg("custom") = CustomRepr(" array([[A, B],\n[C, D]]) "));
m.def(
"kw_lb_func2",
[](const CustomRepr &) {},
py::arg("custom") = CustomRepr("\v\n array([[A, B], [C, D]])"));
m.def(
"kw_lb_func3",
[](const CustomRepr &) {},
py::arg("custom") = CustomRepr("array([[A, B], [C, D]]) \f\n"));
m.def(
"kw_lb_func4",
[](const CustomRepr &) {},
py::arg("custom") = CustomRepr("array([[A, B],\n\f\n[C, D]])"));
m.def(
"kw_lb_func5",
[](const CustomRepr &) {},
py::arg("custom") = CustomRepr("array([[A, B],\r [C, D]])"));
m.def(
"kw_lb_func6", [](const CustomRepr &) {}, py::arg("custom") = CustomRepr(" \v\t "));
m.def(
"kw_lb_func7",
[](const std::string &) {},
py::arg("str_arg") = "First line.\n Second line.");

// test_args_and_kwargs
m.def("args_function", [](py::args args) -> py::tuple {
PYBIND11_WARNING_PUSH
Expand Down
32 changes: 32 additions & 0 deletions tests/test_kwargs_and_defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,38 @@ def test_function_signatures(doc):
doc(m.KWClass.foo1)
== "foo1(self: m.kwargs_and_defaults.KWClass, x: int, y: float) -> None"
)
assert (
doc(m.kw_lb_func0)
== "kw_lb_func0(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
)
assert (
doc(m.kw_lb_func1)
== "kw_lb_func1(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
)
assert (
doc(m.kw_lb_func2)
== "kw_lb_func2(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
)
assert (
doc(m.kw_lb_func3)
== "kw_lb_func3(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
)
assert (
doc(m.kw_lb_func4)
== "kw_lb_func4(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
)
assert (
doc(m.kw_lb_func5)
== "kw_lb_func5(custom: m.kwargs_and_defaults.CustomRepr = array([[A, B], [C, D]])) -> None"
)
assert (
doc(m.kw_lb_func6)
== "kw_lb_func6(custom: m.kwargs_and_defaults.CustomRepr = ) -> None"
)
assert (
doc(m.kw_lb_func7)
== "kw_lb_func7(str_arg: str = 'First line.\\n Second line.') -> None"
)


def test_named_arguments():
Expand Down

0 comments on commit b9359ce

Please sign in to comment.