Replace tunable type selector caster with typing Optional

Author: virtuald

subprojects/robotpy-tunables/semiwrap/TunableTable.yml

@@ -32,8 +32,8 @@ classes:
                    "retained Python values and callbacks."))
       .def("add", &wpi::tunables::python::table::Add,
            py::arg("name"), py::arg("value"), py::kw_only(),
-           py::arg("value_type") = std::nullopt,
-           py::arg("element_type") = std::nullopt,
+           py::arg("value_type") = py::none(),
+           py::arg("element_type") = py::none(),
            py::arg("robust") = false, py::arg("mutable") = true,
            py::arg("on_tune") = std::nullopt,
            py::arg("properties") = std::nullopt,
@@ -83,8 +83,8 @@ classes:
                    "on_tune receives the stored float value."))
       .def("publish_value", &wpi::tunables::python::table::PublishValue,
            py::arg("name"), py::arg("getter"), py::arg("setter"),
-           py::kw_only(), py::arg("value_type") = std::nullopt,
-           py::arg("element_type") = std::nullopt,
+           py::kw_only(), py::arg("value_type") = py::none(),
+           py::arg("element_type") = py::none(),
            py::arg("robust") = false, py::arg("mutable") = true,
            py::arg("properties") = std::nullopt,
            py::arg("type_string") = "",

subprojects/robotpy-tunables/tests/test_tunable.py

@@ -149,21 +149,66 @@ def test_tunable_get_set():
     assert value.get() == 2
 
 
-def test_tunable_type_selectors_use_python_types():
-    integer = tunables.Tunable(1, value_type=int)
-    strings = tunables.Tunable([], element_type=str)
+@pytest.fixture(params=["constructor", "add", "table_add", "publish_value"])
+def create_tunable(backend, request):
+    count = 0
+
+    def create(value, **kwargs):
+        nonlocal count
+        count += 1
+        name = f"value{count}"
+        if request.param == "constructor":
+            return tunables.Tunable(value, **kwargs)
+        if request.param == "add":
+            return tunables.add(name, value, **kwargs)
+        table = tunables.get_table("table")
+        if request.param == "table_add":
+            return table.add(name, value, **kwargs)
+        return table.publish_value(name, lambda: value, lambda value: None, **kwargs)
+
+    return create
+
+
+def test_tunable_type_selectors_use_python_types(create_tunable):
+    integer = create_tunable(1, value_type=int)
+    strings = create_tunable([], element_type=str)
 
     assert integer.get() == 1
     assert strings.get() == []
 
     with pytest.raises(TypeError, match="value_type must be a Python type"):
-        tunables.Tunable(1, value_type="integer")
+        create_tunable(1, value_type="integer")
 
     with pytest.raises(TypeError, match="element_type must be a Python type"):
-        tunables.Tunable([], element_type="string")
+        create_tunable([], element_type="string")
 
     with pytest.raises(TypeError, match="use element_type for sequences"):
-        tunables.Tunable([], value_type=str)
+        create_tunable([], value_type=str)
+
+
+@pytest.mark.parametrize("initial", [1, [1, 2], TunablePoint(1, 2)])
+def test_none_type_selectors_infer_tunable_type(create_tunable, initial):
+    value = create_tunable(initial, value_type=None, element_type=None)
+
+    assert value.get() == initial
+
+
+def test_tunable_type_selectors_are_mutually_exclusive(create_tunable):
+    with pytest.raises(
+        TypeError, match="value_type and element_type are mutually exclusive"
+    ):
+        create_tunable([1], value_type=int, element_type=int)
+
+
+def test_tunable_type_selector_annotations():
+    for method in (
+        tunables.Tunable.__init__,
+        tunables.add,
+        tunables.TunableTable.add,
+        tunables.TunableTable.publish_value,
+    ):
+        assert "value_type: type[object] | None = None" in method.__doc__
+        assert "element_type: type[object] | None = None" in method.__doc__
 
 
 def test_backend_updates_tunables(backend):

subprojects/robotpy-tunables/tunables/src/rpy/PyTunable.cpp

@@ -161,25 +161,26 @@ ValueKind InferSequenceKind(const py::sequence& value) {
 }
 
 ValueKind InferValueKind(
-    py::handle value, const std::optional<PyTunable::PythonType>& valueType,
-    const std::optional<PyTunable::PythonType>& elementType) {
-  if (valueType && elementType) {
+    py::handle value,
+    const py::typing::Optional<PyTunable::PythonType>& valueType,
+    const py::typing::Optional<PyTunable::PythonType>& elementType) {
+  if (!valueType.is_none() && !elementType.is_none()) {
     throw py::type_error("value_type and element_type are mutually exclusive");
   }
-  if (elementType) {
+  if (!elementType.is_none()) {
     if (!IsSequenceValue(value)) {
       throw py::type_error(
           "element_type is only supported for tunable sequences");
     }
-    return KindFromElementType(*elementType);
+    return KindFromElementType(elementType);
   }
-  if (valueType) {
+  if (!valueType.is_none()) {
     if (IsSequenceValue(value)) {
       throw py::type_error(
           "value_type is only supported for scalar tunables; use "
           "element_type for sequences");
     }
-    return KindFromScalarType(*valueType);
+    return KindFromScalarType(valueType);
   }
   if (py::isinstance<py::bool_>(value)) {
     return ValueKind::BOOLEAN;
@@ -210,8 +211,8 @@ ValueKind InferValueKind(
 PyTunable::PyTunable(py::object value, std::optional<Getter> getter,
                      std::optional<Setter> setter,
                      std::optional<TuneCallback> onTune, bool robust,
-                     bool isMutable, std::optional<PythonType> valueType,
-                     std::optional<PythonType> elementType,
+                     bool isMutable, py::typing::Optional<PythonType> valueType,
+                     py::typing::Optional<PythonType> elementType,
                      std::optional<Properties> properties,
                      std::string typeString, bool alwaysGet, bool narrowScalar)
     : m_getter{std::move(getter)},
@@ -436,8 +437,8 @@ wpi::tunables::TunableConfig PyTunable::MakeConfig(
 
 PyTunable::TunableVariant PyTunable::MakeValue(
     py::handle value, bool robust, bool isMutable,
-    const std::optional<PythonType>& valueType,
-    const std::optional<PythonType>& elementType,
+    const py::typing::Optional<PythonType>& valueType,
+    const py::typing::Optional<PythonType>& elementType,
     const std::optional<Properties>& properties, std::string typeString,
     bool alwaysGet, bool narrowScalar) {
   auto kind = InferValueKind(value, valueType, elementType);
@@ -478,8 +479,8 @@ PyTunable::TunableVariant PyTunable::MakeValue(
       return wpi::tunables::TunableStringVector{
           value.cast<std::vector<std::string>>(), config};
     case ValueKind::STRUCT: {
-      py::type type = valueType && IsWpiStructType(*valueType)
-                          ? py::reinterpret_borrow<py::type>(*valueType)
+      py::type type = !valueType.is_none() && IsWpiStructType(valueType)
+                          ? py::reinterpret_borrow<py::type>(valueType)
                           : py::type::of(value);
       int isInstance = PyObject_IsInstance(value.ptr(), type.ptr());
       if (isInstance < 0) {
@@ -496,8 +497,8 @@ PyTunable::TunableVariant PyTunable::MakeValue(
     }
     case ValueKind::STRUCT_ARRAY: {
       auto sequence = py::reinterpret_borrow<py::sequence>(value);
-      py::type type = elementType && IsWpiStructType(*elementType)
-                          ? py::reinterpret_borrow<py::type>(*elementType)
+      py::type type = !elementType.is_none() && IsWpiStructType(elementType)
+                          ? py::reinterpret_borrow<py::type>(elementType)
                           : GetStructSequenceType(sequence);
       ValidateStructSequenceType(sequence, type);
       WPyStructInfo info{type};

subprojects/robotpy-tunables/tunables/src/rpy/PyTunable.h

@@ -16,28 +16,6 @@
 #include "wpi/tunables/Tunable.hpp"
 #include "wpystruct.h"
 
-namespace pybind11::detail {
-
-template <>
-struct type_caster<
-    std::optional<pybind11::typing::Type<pybind11::object>>>
-    : optional_caster<
-          std::optional<pybind11::typing::Type<pybind11::object>>> {
-  bool load(handle src, bool) {
-    if (!src) {
-      return false;
-    }
-    if (src.is_none()) {
-      return true;
-    }
-    value.emplace(pybind11::reinterpret_borrow<
-                  pybind11::typing::Type<pybind11::object>>(src));
-    return true;
-  }
-};
-
-}  // namespace pybind11::detail
-
 namespace wpi::tunables::python {
 
 /**
@@ -80,8 +58,8 @@ class PyTunable : public std::enable_shared_from_this<PyTunable> {
             std::optional<Setter> setter = std::nullopt,
             std::optional<TuneCallback> onTune = std::nullopt,
             bool robust = false, bool isMutable = true,
-            std::optional<PythonType> valueType = std::nullopt,
-            std::optional<PythonType> elementType = std::nullopt,
+            pybind11::typing::Optional<PythonType> valueType = pybind11::none(),
+            pybind11::typing::Optional<PythonType> elementType = pybind11::none(),
             std::optional<Properties> properties = std::nullopt,
             std::string typeString = "", bool alwaysGet = false,
             bool narrowScalar = false);
@@ -173,8 +151,8 @@ class PyTunable : public std::enable_shared_from_this<PyTunable> {
       bool alwaysGet);
   TunableVariant MakeValue(
       pybind11::handle value, bool robust, bool isMutable,
-      const std::optional<PythonType>& valueType,
-      const std::optional<PythonType>& elementType,
+      const pybind11::typing::Optional<PythonType>& valueType,
+      const pybind11::typing::Optional<PythonType>& elementType,
       const std::optional<Properties>& properties, std::string typeString,
       bool alwaysGet, bool narrowScalar);