Skip to content

py::isinstance<py::str>(py_bytes_object) should not return true #1461

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

Closed
jbarlow83 opened this issue Jul 17, 2018 · 2 comments · May be fixed by #1463
Closed

py::isinstance<py::str>(py_bytes_object) should not return true #1461

jbarlow83 opened this issue Jul 17, 2018 · 2 comments · May be fixed by #1463

Comments

@jbarlow83
Copy link
Contributor

Issue description

In pybind 2.2.3, the template specialization py::isinstance<py::str>() returns True when used on py::bytes objects. This is incorrect in both Py2 and 3 (since py::str is unicode in Py2).

py::instance(obj, py::str().get_type()) works correctly.

Reproducible example code

    m.def("test_isinstance",
        []() {
            py::bytes b = "123";
            py::print(py::isinstance<py::str>(b));
        }
    );
>>> pybind11.test_isinstance()
True
@jagerman
Copy link
Member

The reason for the failure is that py::str's check function permissively allows both str and bytes objects (or under Python 2: both unicode and str objects). This is mainly here for Python 2 compatibility to allow str objects to be automatically converted to unicode objects.

We can easily add a isinstance<str> specialization does only checks against str (Py 3) or unicode (Py 2) to keep the up-converting behaviour and fix isinstance. That's going to work as expected under Python 3, but the behaviour under Python 2 is going to be a little strange:

m.def("f", [](py::object o) { return py::isinstance<py::str>(o); });

will result in:

m.f("abc")

returning True under Python 3 but False under Python 2, which might be a little surprising.

I don't think it's necessarily wrong for isinstance to work that way; just perhaps surprising.

@bstaletic
Copy link
Collaborator

Will be fixed in #2256

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

Successfully merging a pull request may close this issue.

4 participants