-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix(types): remove newlines from docstring signature #4735
Changes from 25 commits
c89e5b2
214b200
99cd8d0
f795b7e
9640613
293e6e2
d8e7d2d
418bdef
4521195
d47aa39
7a9382a
15196d1
5d7e555
a2d675f
035a044
19a8d51
122e15a
e6be9d5
1b5a3d7
4c844d5
9918f22
25c9657
0ea0a13
a2add0d
1a9dd7b
d53892c
9cefda8
68c5643
d3885da
b391386
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,37 @@ 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; | ||
bool previous_is_whitespace = false; | ||
|
||
// Replace characters in whitespaces array with spaces and squash consecutive spaces | ||
while (*text != '\0') { | ||
if (strchr(whitespaces, *text)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please make this Elsewhere we have |
||
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 | ||
|
@@ -424,7 +455,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 * | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome that this works on all platforms. But this is a very large Could you please try if it still works like this:
|
||
} 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) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please sprinkle
\r
,\f
,\v
into the tests below?What we want is that a test fails if any of the characters here is accidentally lost. (Imagine a silly typo slipping in while refactoring.)