From 28492edc8358f3e06227ac99026161615d8e7613 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Thu, 18 May 2023 15:22:07 -0700
Subject: [PATCH] Change "format_descriptor_format" implementation as suggested
 by @Lalaland. Additional tests meant to ensure consistency between
 py::format_descriptor<>, np.array, np.format_parser turn out to be useful
 only to highlight long-standing inconsistencies.

---
 tests/test_buffers.cpp | 48 ++++++++++++++++-----------------
 tests/test_buffers.py  | 61 ++++++++++++++++++++++++++++--------------
 2 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/tests/test_buffers.cpp b/tests/test_buffers.cpp
index 24fa46232..c842f5fd5 100644
--- a/tests/test_buffers.cpp
+++ b/tests/test_buffers.cpp
@@ -14,33 +14,33 @@
 #include "pybind11_tests.h"
 
 TEST_SUBMODULE(buffers, m) {
-
-#define PYBIND11_LOCAL_DEF(...)                                                                   \
-    if (cpp_name == #__VA_ARGS__)                                                                 \
-        return py::format_descriptor<__VA_ARGS__>::format();
-
     m.def("format_descriptor_format", [](const std::string &cpp_name) {
-        PYBIND11_LOCAL_DEF(PyObject *)
-        PYBIND11_LOCAL_DEF(bool)
-        PYBIND11_LOCAL_DEF(std::int8_t)
-        PYBIND11_LOCAL_DEF(std::uint8_t)
-        PYBIND11_LOCAL_DEF(std::int16_t)
-        PYBIND11_LOCAL_DEF(std::uint16_t)
-        PYBIND11_LOCAL_DEF(std::int32_t)
-        PYBIND11_LOCAL_DEF(std::uint32_t)
-        PYBIND11_LOCAL_DEF(std::int64_t)
-        PYBIND11_LOCAL_DEF(std::uint64_t)
-        PYBIND11_LOCAL_DEF(float)
-        PYBIND11_LOCAL_DEF(double)
-        PYBIND11_LOCAL_DEF(long double)
-        PYBIND11_LOCAL_DEF(std::complex<float>)
-        PYBIND11_LOCAL_DEF(std::complex<double>)
-        PYBIND11_LOCAL_DEF(std::complex<long double>)
-        return std::string("UNKNOWN");
+        // https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables
+        static auto table = new std::map<std::string, std::string>;
+        if (table->empty()) {
+#define PYBIND11_ASSIGN_HELPER(...)                                                               \
+    (*table)[#__VA_ARGS__] = py::format_descriptor<__VA_ARGS__>::format();
+            PYBIND11_ASSIGN_HELPER(PyObject *)
+            PYBIND11_ASSIGN_HELPER(bool)
+            PYBIND11_ASSIGN_HELPER(std::int8_t)
+            PYBIND11_ASSIGN_HELPER(std::uint8_t)
+            PYBIND11_ASSIGN_HELPER(std::int16_t)
+            PYBIND11_ASSIGN_HELPER(std::uint16_t)
+            PYBIND11_ASSIGN_HELPER(std::int32_t)
+            PYBIND11_ASSIGN_HELPER(std::uint32_t)
+            PYBIND11_ASSIGN_HELPER(std::int64_t)
+            PYBIND11_ASSIGN_HELPER(std::uint64_t)
+            PYBIND11_ASSIGN_HELPER(float)
+            PYBIND11_ASSIGN_HELPER(double)
+            PYBIND11_ASSIGN_HELPER(long double)
+            PYBIND11_ASSIGN_HELPER(std::complex<float>)
+            PYBIND11_ASSIGN_HELPER(std::complex<double>)
+            PYBIND11_ASSIGN_HELPER(std::complex<long double>)
+#undef PYBIND11_ASSIGN_HELPER
+        }
+        return (*table)[cpp_name];
     });
 
-#undef PYBIND11_LOCAL_DEF
-
     // test_from_python / test_to_python:
     class Matrix {
     public:
diff --git a/tests/test_buffers.py b/tests/test_buffers.py
index 943699c0d..c2795e1b7 100644
--- a/tests/test_buffers.py
+++ b/tests/test_buffers.py
@@ -12,29 +12,50 @@ np = pytest.importorskip("numpy")
 
 
 @pytest.mark.parametrize(
-    ("cpp_name", "expected_codes"),
+    ("cpp_name", "expected_fmts", "np_array_dtype"),
     [
-        ("PyObject *", ["O"]),
-        ("bool", ["?"]),
-        ("std::int8_t", ["b"]),
-        ("std::uint8_t", ["B"]),
-        ("std::int16_t", ["h"]),
-        ("std::uint16_t", ["H"]),
-        ("std::int32_t", ["i"]),
-        ("std::uint32_t", ["I"]),
-        ("std::int64_t", ["q"]),
-        ("std::uint64_t", ["Q"]),
-        ("float", ["f"]),
-        ("double", ["d"]),
-        ("long double", ["g", "d"]),
-        ("std::complex<float>", ["Zf"]),
-        ("std::complex<double>", ["Zd"]),
-        ("std::complex<long double>", ["Zg", "Zd"]),
-        ("", ["UNKNOWN"]),
+        ("PyObject *", ["O"], object),
+        ("bool", ["?"], np.bool_),
+        ("std::int8_t", ["b"], np.int8),
+        ("std::uint8_t", ["B"], np.uint8),
+        ("std::int16_t", ["h"], np.int16),
+        ("std::uint16_t", ["H"], np.uint16),
+        ("std::int32_t", ["i"], np.int32),
+        ("std::uint32_t", ["I"], np.uint32),
+        ("std::int64_t", ["q"], np.int64),
+        ("std::uint64_t", ["Q"], np.uint64),
+        ("float", ["f"], np.float32),
+        ("double", ["d"], np.float64),
+        ("long double", ["g", "d"], np.float128),
+        ("std::complex<float>", ["Zf"], np.complex64),
+        ("std::complex<double>", ["Zd"], np.complex128),
+        ("std::complex<long double>", ["Zg", "Zd"], np.complex256),
     ],
 )
-def test_format_descriptor_format(cpp_name, expected_codes):
-    assert m.format_descriptor_format(cpp_name) in expected_codes
+def test_format_descriptor_format(cpp_name, expected_fmts, np_array_dtype):
+    fmt = m.format_descriptor_format(cpp_name)
+    assert fmt in expected_fmts
+
+    # Everything below just documents long-standing inconsistencies.
+    # See also: https://github.com/pybind/pybind11/issues/1908
+
+    # py::format_descriptor<> vs np.array:
+    na = np.array([], dtype=np_array_dtype)
+    bi = m.get_buffer_info(na)
+    if fmt == "q":
+        assert bi.format in ["q", "l"]
+    elif fmt == "Q":
+        assert bi.format in ["Q", "L"]
+    else:
+        assert bi.format == fmt
+
+    # py::format_descriptor<> vs np.format_parser():
+    fmtp = fmt[1:] if fmt.startswith("Z") else fmt
+    fp = np.format_parser(fmtp, [], [])
+    assert fp.dtype is not None
+
+    # DO NOT try to compare fp.dtype and na.dtype, unless you have a lot of
+    # spare time to make sense of it and possibly chime in under #1908.
 
 
 def test_from_python():