Skip to content
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

[BUG]: nb::arg().none() doesn't work with def_prop_rw and primitive types #736

Closed
ghorn opened this issue Sep 27, 2024 · 1 comment
Closed

Comments

@ghorn
Copy link

ghorn commented Sep 27, 2024

Problem description

I'm trying to bind some classes with optional fields. I need to support mutable semantics (rv_policy::reference_internal across the board). So I'm implementing bindings like this:

struct Foo {
  std::optional<Bar> class_field;
  std::optional<int32_t> prim_field;
};

nb::class_<Bar>(m, "Bar").def(nb::init<>());

nb::class_<Foo>(m, "Foo")
  .def(nb::init<>())
  .def_prop_rw(
    "class_field",
    // Bar getter
    [](Foo& value) -> Bar* {
      if (value.class_field.has_value()) {
        return &value.class_field.value();
      }
      return nullptr;
    },
    // Bar setter
    [](Foo& value, Bar* field_value) -> void {
      if (field_value == nullptr) {
        value.class_field = std::nullopt;
      } else {
        value.class_field = std::make_optional(*field_value);
      }
    },
    nb::arg().none())
  .def_prop_rw(
    "prim_field",
    // int32_t getter
    [](Foo& value) -> int32_t* {
      if (value.prim_field.has_value()) {
        return &value.prim_field.value();
      }
      return nullptr;
    },
    // int32_t setter
    [](Foo& value, int32_t* field_value) -> void {
      if (field_value == nullptr) {
        value.prim_field = std::nullopt;
      } else {
        value.prim_field = std::make_optional(*field_value);
      }
    },
    nb::arg().none());

example python code:

  foo = Foo()

  # class field
  assert foo.class_field is None  # works
  foo.class_field = Bar()         # works
  foo.class_field = None          # works

  # primitive field
  assert foo.prim_field is None  # works
  foo.prim_field = 22            # works
  foo.prim_field = None          # *** FAILS! ***

the last line fails with this error:

       TypeError: (): incompatible function arguments. The following argument types are supported:
           1. (self, arg: int | None, /) -> None
       
       Invoked with types: Foo, NoneType

I'm not sure why it is confusing NoneType with None. This is reproducible in 1.9 and 2.1

I've tried putting nb::for_setter(nb::arg().none()) instead of without for_setter. I have two workarounds:

  1. Pass a nb::handle instead of int32_t*. The disadvantage is losing nice error messages and adding error handling code.
  2. Pass optional<int32_t> and use the optional type_caster. The disadvantage is I'd rather not bring that type_caster into scope, because it's so important I don't accidentally use it for non-primitive cases.

I'm curious if this is actually a bug

Reproducible example code

No response

@wjakob
Copy link
Owner

wjakob commented Sep 27, 2024

Type casters for primitives don't accept None, see the documentation. You will have to use std::optional if you want to do this.

@wjakob wjakob closed this as completed Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants