py::str x = py::none() is a bit confusing; it converts None to a string, rather than setting the value to None

[EricCousineau-TRI]

Issue description

If I have a C++ function with py::str as an argument, I wanted to be able to default it to py::none(). However, I was later surprised in my Python code because I was getting 'None' (a string version) rather than purely None.

Reproducible example code

See this patch here:
9c9b1f5

The test fails with the following:

    def test_str_with_cpp_default_none():                                                              
>       assert m.test_str_with_cpp_default_none() == (None, "Hello")                                   
E       AssertionError: assert ('None', 'Hello') == (None, 'Hello')                                    
E         At index 0 diff: 'None' != None

I got bit in the following downstream code:
https://github.com/RobotLocomotion/drake/blob/feb254ea0a711c6596d7b8bf720672e0a5acd329/bindings/pydrake/common/deprecation_pybind.h#L21-L22

The workarounds I could think of:

• Use py::object, and then let Python code enforce the safe of string-ification.
• Use std::optional<std::string> to make it more C++-y

Suggestions

Two options:

• Somehow distinguish assignment-at-construction py::str x = py::none() (use None), assignment py::str x; x = py::none() and make these set the object truly to None; for conversion, py::str(py::none()), allow this to be stringified.
  • I doubt this is possible in C++ without doing weird implicit converting operators.
• Warn about this in the docs here: https://pybind11.readthedocs.io/en/stable/advanced/pycpp/object.html#
  • Something like "Warning about assigning py::none() to objects in C++; This may happen with defautl arguments ...".

[EricCousineau-TRI]

FWIW Trying to write a function like this yields a runtime TypeError, which is good:
2ed6208

m.def("test_str_with_default_arg_none" ,[](py::str value) {
        return value;
    }, py::arg("value") = py::none());

>>> m.test_str_with_default_arg_none()
E       TypeError: test_str_with_default_arg_none(): incompatible function arguments. The following argument types are supported:
E           1. (value: str = None) -> str
E       
E       Invoked with:

See add'l commit:

[YannickJadoul]

None is not a str, so it's understandable if it wouldn't accept it, right?
The conversion is rather surprising, I agree.

@rwgk, more fun with strings?

[EricCousineau-TRI]

Yeah... I think it'd be hard to enforce anything explicit in C++ code, so I think a Warning in the Python C++ API docs, with an expansion below, would suffice?

I'ma submit a quick draft PR anwyho.

[rwgk]

The Python 3 interpreter turns str(None) into None, matching pybind11::str(py::none()).
What's surprising is the assignment operator, which in the interpreter changes the type of the assignee from str to NoneType, which we cannot do / do not want to in C++.
In theory we could raise an exception from the C++ operator= when we see any operand type other than str.
But then again, do we want to allow py::str x = 42?
I'd so "No", you have to write py::str x(42) if you want the conversion.

[EricCousineau-TRI]

But then again, do we want to allow py::str x = 42?
I'd so "No", you have to write py::str x(42) if you want the conversion.

I'm just a bit concerned that we won't be able to express a difference between py::str x = 42 and py::str x(42). AFAIK, py::str x = 42 will invoke the same code path to construct it (the constructor).

Will need to confirm this, though, with a quick example...

[EricCousineau-TRI]

The Python 3 interpreter turns str(None) into None, matching pybind11::str(py::none()).
What's surprising is the assignment operator, which in the interpreter changes the type of the assignee from str to NoneType, which we cannot do / do not want to in C++.

This part, I wasn't sure of. I kind of thought of py::str as an extension to py::object, which can hold None if you think of it as a pointer, similar to what you'd have with Java's String or C#'s string, etc. (Strictly typed, but nullable)

[rwgk]

py::str x = 42 will invoke the same code path to construct it (the constructor).

Oh, sorry, I think you're right (pretty sure now, I just looked around a bit to refresh my understanding).
Then there isn't much we can do.

[EricCousineau-TRI]

Yeah :( I think docs are the main route...

FWIW Here're some cppreference links:

• https://en.cppreference.com/w/cpp/language/cast_operator#Explanation
• https://en.cppreference.com/w/cpp/language/operators - ironically, they don't reference constructor precedence at all, only when conversion gets invoked as a side effect :(

[rwgk]

The Python 3 interpreter turns str(None) into None, matching pybind11::str(py::none()).
What's surprising is the assignment operator, which in the interpreter changes the type of the assignee from str to NoneType, which we cannot do / do not want to in C++.

This part, I wasn't sure of. I kind of thought of py::str as an extension to py::object, which can hold None if you think of it as a pointer, similar to what you'd have with Java's String or C#'s string, etc. (Strictly typed, but nullable)

I just tried this:

m.def("junk", []() { py::int_ i = py::none(); });

This raises (I only tried Python 3):

TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType'

[EricCousineau-TRI]

Yeah, for certain types, it will fail fast (which is great); I have a test at least showing py::str = py::none() (does not fail fast) and py::dict = py::none() (failing fast) here:
https://github.com/pybind/pybind11/pull/2362/files#diff-0b3f8a97d31285037ace0a15a3acc04c

[YannickJadoul]

Actually, I can't reproduce, @EricCousineau-TRI

#include <pybind11/pybind11.h>

namespace py = pybind11;

PYBIND11_MODULE(example, m)
{
        m.def("f" ,[](py::str value) {
                return value;
        });
}

$ python3.8
Python 3.8.0 (default, Oct 28 2019, 16:14:01) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import example
>>> example.f(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: f(): incompatible function arguments. The following argument types are supported:
    1. (arg0: str) -> str

Invoked with: None

Am I missing something? Is this dependent on the right Python version?

[YannickJadoul]

Alright, sorry. Misread and didn't open your commit. This is a pure C++ issue, then, where

#define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ConvertFun) \
    PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
    /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \
    Name(const object &o) \
    : Parent(check_(o) ? o.inc_ref().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
    { if (!m_ptr) throw error_already_set(); } \
    Name(object &&o) \
    : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
    { if (!m_ptr) throw error_already_set(); } \
    template <typename Policy_> \
    Name(const ::pybind11::detail::accessor<Policy_> &a) : Name(object(a)) { }

defines an implicit conversion (willingly and knowingly so, given the comment).

[EricCousineau-TRI]

Yeah... I've tried to encode that in the doc writeup + unittests in #2362