Dtype kind vs char #2864

bemichel · 2021-02-17T15:04:17Z

#2860 Description

Suggested changelog entry:

* NumPy: added ``.char_()`` to type which gives the NumPy public ``char`` result, which also distinguishes types by bit length (unlike ``.kind()``).

…(eq. to dtype.char in Python)

…hon numpy interface

YannickJadoul

Nice, thanks, @bemichel! :-)

YannickJadoul · 2021-02-17T15:18:31Z

include/pybind11/numpy.h

@@ -507,11 +507,16 @@ class dtype : public object {
        return detail::array_descriptor_proxy(m_ptr)->names != nullptr;
    }

-    /// Single-character type code.
+    /// Single-character for dtype's kind (ex: float and double are 'f' or int and long int are 'i')


"float and double" could be "floating point types", and "int and long" could be "integral types". But this works as well.

YannickJadoul · 2021-02-17T15:20:35Z

include/pybind11/numpy.h

    char kind() const {
        return detail::array_descriptor_proxy(m_ptr)->kind;
    }

+    /// Single-character for dtype's type (ex: float is 'f' and double 'd')
+    char char_() const {
+        return detail::array_descriptor_proxy(m_ptr)->type;


Is it worth adding a comment that we're following the Python API of dtype (i.e., dtype.char), rather than NumPy's internal C API (PyArray_Descr::type), for the name?

See also https://numpy.org/devdocs/reference/c-api/types-and-structures.html#c.PyArray_Descr for the root cause of the type vs. char confusion.

tests/test_numpy_dtypes.py

bemichel · 2021-02-17T17:38:04Z

Hi,
I do not understand why tests are not passed : it seems a cmake error in the report ???

…::type to a plain, non-doxygen comment

YannickJadoul · 2021-02-17T17:51:03Z

Hi,
I do not understand why tests are not passed : it seems a cmake error in the report ???

No worries. Our CMake is broken, but is getting fixed in #2865. I'll rerun the tests once that passes

bemichel · 2021-02-17T17:58:42Z

Okay !
Let me known if this last commit satisfy your demand ?
Bertrand.

YannickJadoul · 2021-02-17T18:51:42Z

Let's restart those tests :-)

Also, I had a few more proposed changes, but let me push them.

YannickJadoul

@bemichel, I still proposed a few changes in 400ffea, but if those are fine with you, I'm perfectly happy with this PR.

bstaletic

@YannickJadoul I know you're not a fan of Windoze, but this PR breaks that OS.

bstaletic · 2021-02-17T19:20:51Z

tests/test_numpy_dtypes.cpp

+                     py::dtype("int64"),   // long int
+                     py::dtype("float32"), // float
+                     py::dtype("float64"), // double
+                     py::dtype("float128") // long double


On Winblows sizeof(long double) == sizeof(double), which might explain the error.

I was about to say: MSVC doesn't really like this, @bemichel.

Also, those comments are not completely right: sometimes np.int32 == np.int, but sometimes np.int64 == np.int. It really depends on the platform, I think?

But then, as far as I remember (I once contributed to this part of NumPy's docstrings in a sprint at a conference), the letters are actually linked to np.bool, np.int, np.float`, etc, and not to the sized variants!

~~Scratch that. Seems that either they changed, or I misremember.~~

No, I was right. But np.int is an alias for a Python int, not for a C int, that's np.intc.

At any rate, here are the docs:
https://numpy.org/doc/stable/reference/arrays.scalars.html#built-in-scalar-types

intN or floatN are aliases, so I'd suggest using the non-alias versions.

Actually. It's easier if I just try fixing this. Let's see.

Done. Let's see what this says and does.

Types in tests

YannickJadoul · 2021-02-17T20:40:36Z

OK, things are working. Want to check if you agree with the changes I made, @bemichel?

bemichel · 2021-02-18T08:35:03Z

@bemichel, I still proposed a few changes in 400ffea, but if those are fine with you, I'm perfectly happy with this PR.

Perfect !

bemichel · 2021-02-18T08:37:14Z

I agree with everything !!! Great idea to factorize the list of "C/C++" types.
Bertrand.

YannickJadoul · 2021-02-18T08:56:22Z

Great; let's see who else will review then, before this is done :-)

YannickJadoul · 2021-02-23T09:57:08Z

Great; let's see who else will review then, before this is done :-)

Seems that was @bstaletic :-)

Thanks, @bemichel!

v2.7.0 (Jul 16, 2021) --------------------- New features: * Enable ``py::implicitly_convertible<py::none, ...>`` for ``py::class_``-wrapped types. `#3059 <https://github.com/pybind/pybind11/pull/3059>`_ * Allow function pointer extraction from overloaded functions. `#2944 <https://github.com/pybind/pybind11/pull/2944>`_ * NumPy: added ``.char_()`` to type which gives the NumPy public ``char`` result, which also distinguishes types by bit length (unlike ``.kind()``). `#2864 <https://github.com/pybind/pybind11/pull/2864>`_ * Add ``pybind11::bytearray`` to manipulate ``bytearray`` similar to ``bytes``. `#2799 <https://github.com/pybind/pybind11/pull/2799>`_ * ``pybind11/stl/filesystem.h`` registers a type caster that, on C++17/Python 3.6+, converts ``std::filesystem::path`` to ``pathlib.Path`` and any ``os.PathLike`` to ``std::filesystem::path``. `#2730 <https://github.com/pybind/pybind11/pull/2730>`_ * A ``PYBIND11_VERSION_HEX`` define was added, similar to ``PY_VERSION_HEX``. `#3120 <https://github.com/pybind/pybind11/pull/3120>`_ Signed-off-by: Zheng Ruoqin <[email protected]> Signed-off-by: Khem Raj <[email protected]>

v2.7.0 (Jul 16, 2021) --------------------- New features: * Enable ``py::implicitly_convertible<py::none, ...>`` for ``py::class_``-wrapped types. `#3059 <https://github.com/pybind/pybind11/pull/3059>`_ * Allow function pointer extraction from overloaded functions. `#2944 <https://github.com/pybind/pybind11/pull/2944>`_ * NumPy: added ``.char_()`` to type which gives the NumPy public ``char`` result, which also distinguishes types by bit length (unlike ``.kind()``). `#2864 <https://github.com/pybind/pybind11/pull/2864>`_ * Add ``pybind11::bytearray`` to manipulate ``bytearray`` similar to ``bytes``. `#2799 <https://github.com/pybind/pybind11/pull/2799>`_ * ``pybind11/stl/filesystem.h`` registers a type caster that, on C++17/Python 3.6+, converts ``std::filesystem::path`` to ``pathlib.Path`` and any ``os.PathLike`` to ``std::filesystem::path``. `#2730 <https://github.com/pybind/pybind11/pull/2730>`_ * A ``PYBIND11_VERSION_HEX`` define was added, similar to ``PY_VERSION_HEX``. `#3120 <https://github.com/pybind/pybind11/pull/3120>`_ Signed-off-by: Zheng Ruoqin <[email protected]> Signed-off-by: Khem Raj <[email protected]> Signed-off-by: Trevor Gamblin <[email protected]>

The proximate reason for the upgrade is to pick up a thread-safety fix (pybind/pybind11#3237) for JAX, but we might as well just upgrade to the current version for everyone. Remove a backport of a patch now present in the released version (pybind/pybind11#2864). Patch out the "VERSION" file from nsync to avoid conflicts with the <version> C++ header included by pybind11. PiperOrigin-RevId: 420856622 Change-Id: Ied007ed62b93d96cf008d6ec6336017af1fa10fd

v2.7.0 (Jul 16, 2021) --------------------- New features: * Enable ``py::implicitly_convertible<py::none, ...>`` for ``py::class_``-wrapped types. `#3059 <https://github.com/pybind/pybind11/pull/3059>`_ * Allow function pointer extraction from overloaded functions. `#2944 <https://github.com/pybind/pybind11/pull/2944>`_ * NumPy: added ``.char_()`` to type which gives the NumPy public ``char`` result, which also distinguishes types by bit length (unlike ``.kind()``). `#2864 <https://github.com/pybind/pybind11/pull/2864>`_ * Add ``pybind11::bytearray`` to manipulate ``bytearray`` similar to ``bytes``. `#2799 <https://github.com/pybind/pybind11/pull/2799>`_ * ``pybind11/stl/filesystem.h`` registers a type caster that, on C++17/Python 3.6+, converts ``std::filesystem::path`` to ``pathlib.Path`` and any ``os.PathLike`` to ``std::filesystem::path``. `#2730 <https://github.com/pybind/pybind11/pull/2730>`_ * A ``PYBIND11_VERSION_HEX`` define was added, similar to ``PY_VERSION_HEX``. `#3120 <https://github.com/pybind/pybind11/pull/3120>`_ Signed-off-by: Zheng Ruoqin <[email protected]> Signed-off-by: Khem Raj <[email protected]> Signed-off-by: Trevor Gamblin <[email protected]>

Bertrand MICHEL added 3 commits February 15, 2021 17:17

[dtype]: add type() method to access type attribute of PyArray_Descr …

df1be99

…(eq. to dtype.char in Python)

[dtype] change type() name method to char_() to be compliant with Pyt…

b59299d

…hon numpy interface

[dtype] fix by pre-commit

1400520

YannickJadoul reviewed Feb 17, 2021

View reviewed changes

[dtype] Change comments and solutions format for test

f455222

Clarify documentation and move note about dtype.char vs PyArray_Descr…

400ffea

…::type to a plain, non-doxygen comment

YannickJadoul closed this Feb 17, 2021

YannickJadoul reopened this Feb 17, 2021

YannickJadoul previously approved these changes Feb 17, 2021

View reviewed changes

bstaletic reviewed Feb 17, 2021

View reviewed changes

YannickJadoul added 3 commits February 17, 2021 21:00

Fix and extend tests

f3ba900

Fix the supposedly fixed tests

14c9f7d

Fix the fixed tests again

46701ec

YannickJadoul approved these changes Feb 17, 2021

View reviewed changes

YannickJadoul added this to the v2.7 milestone Feb 18, 2021

bstaletic approved these changes Feb 23, 2021

View reviewed changes

YannickJadoul merged commit 74a767d into pybind:master Feb 23, 2021

github-actions bot added the needs changelog Possibly needs a changelog entry label Feb 23, 2021

henryiii removed the needs changelog Possibly needs a changelog entry label Jul 13, 2021

ax3l mentioned this pull request Jan 13, 2023

Fix dtype_from_numpy openPMD/openPMD-api#1357

Merged

rwgk mentioned this pull request Feb 10, 2023

FWD pybind11 google/pybind11clif#2864

Closed

GiulioRomualdi mentioned this pull request Jul 6, 2023

Camera bridge bindings ami-iit/bipedal-locomotion-framework#695

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dtype kind vs char #2864

Dtype kind vs char #2864

bemichel commented Feb 17, 2021 •

edited by henryiii

Loading

YannickJadoul left a comment

YannickJadoul Feb 17, 2021

bemichel Feb 17, 2021

YannickJadoul Feb 17, 2021

bemichel Feb 17, 2021

bemichel commented Feb 17, 2021

YannickJadoul commented Feb 17, 2021

bemichel commented Feb 17, 2021

YannickJadoul commented Feb 17, 2021

YannickJadoul left a comment

bstaletic left a comment

bstaletic Feb 17, 2021

YannickJadoul Feb 17, 2021

YannickJadoul Feb 17, 2021

YannickJadoul Feb 17, 2021

YannickJadoul Feb 17, 2021

YannickJadoul Feb 17, 2021

YannickJadoul commented Feb 17, 2021

bemichel commented Feb 18, 2021

bemichel commented Feb 18, 2021

YannickJadoul commented Feb 18, 2021

YannickJadoul commented Feb 23, 2021

Dtype kind vs char #2864

Dtype kind vs char #2864

Conversation

bemichel commented Feb 17, 2021 • edited by henryiii Loading

Suggested changelog entry:

YannickJadoul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bemichel commented Feb 17, 2021

YannickJadoul commented Feb 17, 2021

bemichel commented Feb 17, 2021

YannickJadoul commented Feb 17, 2021

YannickJadoul left a comment

Choose a reason for hiding this comment

bstaletic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YannickJadoul commented Feb 17, 2021

bemichel commented Feb 18, 2021

bemichel commented Feb 18, 2021

YannickJadoul commented Feb 18, 2021

YannickJadoul commented Feb 23, 2021

bemichel commented Feb 17, 2021 •

edited by henryiii

Loading