[BUG] Allow for __str__ to be usable with enums (again)

[hkaiser]

In version 2.4 the following code did the expected thing, i.e. invoked the defined __str__ overload for the enum when str() was used in Python code on an instance of that enum:

enum Pet {
    Dog = 0,
    Cat
};

auto pet = py::enum_<Pet>(m, "Pet", py::arithmetic(), "some pet")
    .value("Dog", Dog)
    .value("Cat", Cat)
    .def("__str__", [](Pet p) { return p == Dog ? "Dog" : "Cat"; });

This does not work anymore in V2.6 (I have not checked in V2.5, though). V2.6 always returns either "Pet.Dog" or "Pet.Cat" and ignores the defined __str__.

[sizmailov]

The default implementation of __str__ for enums was added in #2126. When you define __str__ the function gets overloaded against py::handle (default) and Pet. Overloads are tried in order of definition, therefore second one have no chance to be called.

A work-around would be to avoid overloading and replace the default implementation. I think the shortest snippet for this would be

pet.attr("__str__") = py::cpp_function(
    [](Pet &self){ ... },
    py::name("__str__"), 
    py::is_method(pet)
);

This effectively repeats body of .def() method except that it omits sibling(getattr(*this, name_, none())) in construction arguments ofcpp_function, so function gets overwritten, not overloaded.

Probably overriding should be made less verbose than that (e.g. adding def_override method?).

[YannickJadoul]

We could also just make the default __str__ implementation optional, with a switch to the constructor?

[YannickJadoul]

I'm adding this to the 2.6.0 milestone, to see if we want to check this before that release.

[henryiii]

I believe this would be solvable with #1131

[YannickJadoul]

Also, yes. That would be a nice solution as well! :-)

[sizmailov]

@henryiii Using py::prepend() here would be a half-way solution. An overloaded function is still created which might have subtle runtime overhead and can be visible in docstring which doesn't make sense.

@YannickJadoul Disabling default implementations with constructor flags might turn into combinatorical hell if user wants to override e.g. two out of three functions. Or maybe constructor flags should be parametrized with ignored functions...

Looks like a hypotetical py::override() (by analogy with py::prepend() should be more appropriate in this case.

[YannickJadoul]

You're right about @henryiii's and my proposed solution, but I do feel we need a bit more time to consider the consequences of py::override (as well as the naming; we finally fixed PYBIND11_OVERLOAD to PYBIND11_OVERRIDE, but this is yet a different kind of override?), so I don't think it's still very feasible to get into 2.6.0

[henryiii]

I would assume "py::override" would delete the call chain and then add itself? Maybe py::replace? And would it remove the whole call chain, or just a matching signature (is that even possible?). Agree that it would be a post-2.6.0 project.

Isn't the 'bug' here that __str__ didn't exist before but does now; if you wanted to override __repr__, then before and now you can't do so without a workaround? py::prepend is a little bit of a half-way solution, yes, but it does provide a way around a regression in 2.6.0, and we already have a call-chain and adding functions now always appends to it. Without the ability to at least prepend, we can't add new methods to our built-in objects in new releases.

[henryiii]

The call-chain itself always has runtime overhead, but selecting the first item in the chain should be the same, regardless of the length of the chain (1 vs. 2) (unless we optimize something somehow for length-1). The docstring, yes, probably would have two lines.

[sizmailov]

naming

One of the hardest problems in CS :)

I'm not against py::append in general, but it targets a different problem. I totally agree with your arguments about it.

Yes, I think py::override/py::replace should replace whole call chain. The use case for this features seems to be very narrow which weakens it's "merge potential". (I can imagine some sort of reflection on top of pybind11 bindings where it can find another use)

[hkaiser]

The default implementation of __str__ for enums was added in #2126. When you define __str__ the function gets overloaded against py::handle (default) and Pet. Overloads are tried in order of definition, therefore second one have no chance to be called.

A work-around would be to avoid overloading and replace the default implementation. I think the shortest snippet for this would be

pet.attr("__str__") = py::cpp_function(
    [](Pet &self){ ... },
    py::name("__str__"), 
    py::is_method(pet)
);

This effectively repeats body of .def() method except that it omits sibling(getattr(*this, name_, none())) in construction arguments ofcpp_function, so function gets overwritten, not overloaded.

Probably overriding should be made less verbose than that (e.g. adding def_override method?).

FWIW, this workaround does the trick. Thanks!