t option and custom option arguments are tricky to handle correctly

[mooreryan]

So this is sort of a subtly weird thing that can trip you up if you're not careful.

Say you have a python fun like this:

    def say_hi(self, to_whom=None, greeting='hi'):
        if to_whom:
            return(f'{self.name} says {greeting} to {to_whom.name}')
        else:
            return(f'{self.name} says {greeting}')

Given that to_whom takes None in the python, you decide to make it explicit in the OCaml binding.

  val say_hi : t -> to_whom:t option -> greeting:string option -> unit -> string

However, this generates code that has the wrong signature

  let say_hi t ~to_whom ~greeting () =
    let callable = Py.Object.find_attr_string t "say_hi" in
    let kwargs =
      filter_opt
        [
          Some ("to_whom", to_pyobject to_whom);
          Some
            ( "greeting",
              (function Some x -> Py.String.of_string x | None -> Py.none)
                greeting );
        ]
    in
    Py.String.to_string
    @@ Py.Callable.to_function_with_keywords callable [||] kwargs

That sig is val say_hi : t -> to_whom:t -> greeting:string option -> unit -> string.

To make that arg actually optional with t, you would have to do this instead.

  val say_hi2 : t -> ?to_whom:t -> greeting:string option -> unit -> string

Note that this problem is only there for t option and custom option eg Doc.t option.

Fixing it

It's not straightforward to fix as t option as an argument passed to a python function and t option as a return type of an OCaml function need to be treated differently (and that depends on the --of-pyo-ret-type flag).

Short term fix is to document the weirdness and give an error when users try to use t option in this way. Currently, the bindings will be generated and you won't know something is wrong until you try to build it.

Long term is to actually handle t option properly as an arg and also properly as a return type.

[mooreryan]

The other thing to keep in mind is if you have a Python function that returns either a custom object or None, the t option return type will be wrong too.

class Person:
    def __init__(self, name, age):
        self.name = name
        self.age = age

    def maybe_make_friend(self, color):
        if color == 'orange':
            return(Person(f'{self.name}\'s friend, Orange Soda', 33))
        else:
            return(None)

You would think to bind it like this:

  val maybe_make_friend : t -> color:string -> unit -> t option

That generates

  let maybe_make_friend t ~color () =
    let callable = Py.Object.find_attr_string t "maybe_make_friend" in
    let kwargs = filter_opt [ Some ("color", Py.String.of_string color) ] in
    of_pyobject @@ Py.Callable.to_function_with_keywords callable [||] kwargs

When the user is expecting it to generate something more like this:

let f t () =
  let callable = Py.Object.find_attr_string t "f" in
  let kwargs = filter_opt [ ] in
  (fun x -> if Py.is_none x then None else Some (of_pyobject x))
  @@ Py.Callable.to_function_with_keywords callable [||] kwargs

As above it has to do with the way --of-pyo-ret-type is working with things. And the fact that t and custom types are treating differently from something like int option.