`tl::expected` : yay or nay?

[sea-bass]

I really like tl::expected. It makes C++ code really nice to work with.

However, I am worried about making Python bindings if most functions have output types wrapped in expecteds.

Critically, nanobind does not support this (nor should it because it's a C++17 library and this feature is 23? (see discussion)), so we'd have to essentially encase all our bindings into lambdas that check the expected and do a custom thing on the Python side in the event of failure -- like raise an exception.

I'm sure this is macroable, but... is it worth it? I wanted to open up some discussions here.

cc @eholum @v4hn (because I can pick on you)

[eholum]

Why do you ask me these hard questions? I'm just here to poke at inconsequential CI improvements.

So this might be the wrong opinion, but I'd take an error status or error code any day of the week over tl::expected for the high level interfaces. Because I also like tl::expected, but feel like it should be used internally only. Are you thinking you'd slap it on the something like solve?

[sea-bass]

I was thinking of adding it to pretty much everything!

The cool thing about expecteds is that you can then take the error from an internal function and pipe it directly up a call stack.

But on the Python side, perhaps best to grab errors and rethrow them as actual exceptions?

How would return statuses work? All return types would then need to be wrapped into something that combines the return status and the actual intended output? Or are you thinking returning statuses and all the real output is outparameters?

[sea-bass]

Another annoying thing I realize with this is that, if we're supporting non-ROS workflows, it means we'd need to include the tl_expected repo from source... which is fine, it's single-header with a very permissive license.

But this does push me over to explore alternative solutions.

I've started using std::optional<T> in some code, which nanobind binds to T | None, and that's nice.

[eholum]

I've started using std::optional in some code, which nanobind binds to T | None, and that's nice.

This sounds nice and clean but I guess you lose error information if you just get nothing back. Have you tried binding to tl::expected somehow? Is that even possible?

It's easy peasy to add helper functions for the python bindings that just unwrap the expected and throw an exception if there's an error. But then you just have helper functions everywhere and that's a bummer. Or else some global helper function but then we'd have to basically redefine every binding,

template<typename T, typename E>
T unwrap_expected(const tl::expected<T, E>& ret) {
    if (ret.has_value()) {
        return ret.value();
    } else {
        // Do something?
    }
}

EDIT: Ok I was curious enough to see what this looked like so threw it in here:

5b2eb5b

[sea-bass]

Absolute legend.

[sea-bass]

This is one of those places where macros wouldn't necessarily be objectionable, if the helper function approach is still too verbose.

[eholum]

@sea-bass says yay.

Would recommend macro-izing the change above and doing some testing to see how exceptions are handled/propagated from C++ through nanobind.