Add const T to docstring generation. #3020
Conversation
|
Hi @jhale, I think the valgrind error is unrelated (looks like pocketfft suppression needs to be updated again), but could you please add a test that fails without your change, and passes with your change? |
|
I fixed the valgrind error. If you rebase this PR on master the CI should come back clean. |
|
I've added a test that fails on On master the returned docstring for both is: |
tests/test_numpy_array.py
Outdated
| @@ -481,6 +481,9 @@ def test_index_using_ellipsis(): | |||
| a = m.index_using_ellipsis(np.zeros((5, 6, 7))) | |||
| assert a.shape == (6,) | |||
|
|
|||
| def test_format_descriptors_for_const_types(): | |||
There was a problem hiding this comment.
For completeness, could you please add corresponding non-const tests, even if redundant (I didn't look if that's covered elsewhere already). It's usually good to exercise closely related contrasting functionality together, especially if it's super easy, as in this case.
tests/test_numpy_array.cpp
Outdated
| @@ -437,4 +437,10 @@ TEST_SUBMODULE(numpy_array, sm) { | |||
| sm.def("accept_double_f_style_forcecast_noconvert", | |||
| [](py::array_t<double, py::array::forcecast | py::array::f_style>) {}, | |||
| "a"_a.noconvert()); | |||
|
|
|||
| // Check that types returns correct npy format descriptor | |||
| sm.def("float", [](py::array_t<float>) {}); | |||
There was a problem hiding this comment.
Slight worry about clash with built-in float. Could you please prepend array_ (or similar) to all functions to make the names more unique?
There was a problem hiding this comment.
I figured you're in a European time zone, so I went ahead and started working on the prefix. When I looked more closely I decided test_fmt_desc_ is better as a prefix, and while I was at it I also condensed the test and made it less platform-specific.
I also double-checked that the new test fails without your change to the include:
test_numpy_array.py .....................................FF............. [100%]
========================================================================================== FAILURES ==========================================================================================
________________________________________________________ test_format_descriptors_for_floating_point_types[test_fmt_desc_const_float] _________________________________________________________
test_func = <built-in method test_fmt_desc_const_float of PyCapsule object at 0x7fba1ce05c60>
@pytest.mark.parametrize(
"test_func",
[
m.test_fmt_desc_float,
m.test_fmt_desc_double,
m.test_fmt_desc_const_float,
m.test_fmt_desc_const_double,
],
)
def test_format_descriptors_for_floating_point_types(test_func):
> assert "numpy.ndarray[numpy.float" in test_func.__doc__
E AssertionError: assert 'numpy.ndarray[numpy.float' in 'test_fmt_desc_const_float(arg0: numpy.ndarray[numpy.longdouble]) -> None\n'
E + where 'test_fmt_desc_const_float(arg0: numpy.ndarray[numpy.longdouble]) -> None\n' = <built-in method test_fmt_desc_const_float of PyCapsule object at 0x7fba1ce05c60>.__doc__
test_numpy_array.py:495: AssertionError
________________________________________________________ test_format_descriptors_for_floating_point_types[test_fmt_desc_const_double] ________________________________________________________
test_func = <built-in method test_fmt_desc_const_double of PyCapsule object at 0x7fba1ce05c90>
@pytest.mark.parametrize(
"test_func",
[
m.test_fmt_desc_float,
m.test_fmt_desc_double,
m.test_fmt_desc_const_float,
m.test_fmt_desc_const_double,
],
)
def test_format_descriptors_for_floating_point_types(test_func):
> assert "numpy.ndarray[numpy.float" in test_func.__doc__
E AssertionError: assert 'numpy.ndarray[numpy.float' in 'test_fmt_desc_const_double(arg0: numpy.ndarray[numpy.longdouble]) -> None\n'
E + where 'test_fmt_desc_const_double(arg0: numpy.ndarray[numpy.longdouble]) -> None\n' = <built-in method test_fmt_desc_const_double of PyCapsule object at 0x7fba1ce05c90>.__doc__
test_numpy_array.py:495: AssertionError
================================================================================ 2 failed, 50 passed in 0.40s ================================================================================
…nctions even if one fails; 3. be less platform-specific (e.g. C++ float is not necessarily float32).
|
CI looks good, except for the Centos 8 build with is known to be broken for unrelated reasons. I'll merge this PR now. Thanks @jhale for the fix! |
Description
When using const T in pyarray auto-generated documentation shows long double type.
Suggested changelog entry: