Merge pull request #1985 from PyO3/is_instance_of

add PyType::is_subclass_of and PyAny::is_instance_of

CHANGELOG.md

@@ -20,8 +20,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Add `Py::setattr` method. [#2009](https://github.com/PyO3/pyo3/pull/2009)
 
-## Removed
+### Changed
 
+- `PyType::is_subclass`, `PyErr::is_instance` and `PyAny::is_instance` now operate run-time type object instead of a type known at compile-time. The old behavior is still available as `PyType::is_subclass_of`, `PyErr::is_instance_of` and `PyAny::is_instance_of`.  [#1985](https://github.com/PyO3/pyo3/pull/1985)
+
+### Removed
+
+- Remove `PyType::is_instance`, which is unintuitive; instead of `typ.is_instance(obj)`, use `obj.is_instance(typ)`. [#1985](https://github.com/PyO3/pyo3/pull/1985)
 - Remove all functionality deprecated in PyO3 0.14. [#2007](https://github.com/PyO3/pyo3/pull/2007)
 
 ## [0.15.1] - 2021-11-19

guide/src/conversions/traits.md

@@ -413,7 +413,7 @@ enum RustyEnum {
 #         {
 #             let thing = b"foo".to_object(py);
 #             let error = thing.extract::<RustyEnum>(py).unwrap_err();
-#             assert!(error.is_instance::<pyo3::exceptions::PyTypeError>(py));
+#             assert!(error.is_instance_of::<pyo3::exceptions::PyTypeError>(py));
 #         }
 # 
 #         Ok(())

guide/src/ecosystem/async-await.md

@@ -536,12 +536,13 @@ fn main() -> PyResult<()> {
         pyo3_asyncio::async_std::run(py, async move {
             // verify that we are on a uvloop.Loop
             Python::with_gil(|py| -> PyResult<()> {
-                assert!(uvloop
+                assert!(pyo3_asyncio::async_std::get_current_loop(py)?.is_instance(
+                    uvloop
                         .as_ref(py)
                         .getattr("Loop")?
                         .downcast::<PyType>()
                         .unwrap()
-                    .is_instance(pyo3_asyncio::async_std::get_current_loop(py)?)?);
+                )?);
                 Ok(())
             })?;
 

guide/src/exception.md

@@ -100,21 +100,19 @@ PyErr::from_instance(py, err).restore(py);
 ## Checking exception types
 
 Python has an [`isinstance`](https://docs.python.org/3/library/functions.html#isinstance) method to check an object's type.
-In PyO3 every native type has access to the [`PyAny::is_instance`] method which does the same thing.
+In PyO3 every object has the [`PyAny::is_instance`] and [`PyAny::is_instance_of`] methods which do the same thing.
 
 ```rust
 use pyo3::Python;
 use pyo3::types::{PyBool, PyList};
 
 Python::with_gil(|py| {
-    assert!(PyBool::new(py, true).is_instance::<PyBool>().unwrap());
+    assert!(PyBool::new(py, true).is_instance_of::<PyBool>().unwrap());
     let list = PyList::new(py, &[1, 2, 3, 4]);
-    assert!(!list.is_instance::<PyBool>().unwrap());
+    assert!(!list.is_instance_of::<PyBool>().unwrap());
-    assert!(list.is_instance::<PyList>().unwrap());
+    assert!(list.is_instance_of::<PyList>().unwrap());
 });
 ```
-[`PyAny::is_instance`] calls the underlying [`PyType::is_instance`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyType.html#method.is_instance)
-method to do the actual work.
 
 To check the type of an exception, you can similarly do:
 
@@ -123,7 +121,7 @@ To check the type of an exception, you can similarly do:
 # use pyo3::prelude::*;
 # Python::with_gil(|py| {
 # let err = PyTypeError::new_err(());
-err.is_instance::<PyTypeError>(py);
+err.is_instance_of::<PyTypeError>(py);
 # });
 ```
 
@@ -184,7 +182,7 @@ fn main() {
     Python::with_gil(|py| {
         let fun = pyo3::wrap_pyfunction!(connect, py).unwrap();
         let err = fun.call1(("0.0.0.0",)).unwrap_err();
-        assert!(err.is_instance::<PyOSError>(py));
+        assert!(err.is_instance_of::<PyOSError>(py));
     });
 }
 ```
@@ -209,13 +207,13 @@ fn parse_int(s: String) -> PyResult<usize> {
 #
 #         assert!(parse_int(String::from("-1"))
 #             .unwrap_err()
-#             .is_instance::<PyValueError>(py));
+#             .is_instance_of::<PyValueError>(py));
 #         assert!(parse_int(String::from("foo"))
 #             .unwrap_err()
-#             .is_instance::<PyValueError>(py));
+#             .is_instance_of::<PyValueError>(py));
 #         assert!(parse_int(String::from("13.37"))
 #             .unwrap_err()
-#             .is_instance::<PyValueError>(py));
+#             .is_instance_of::<PyValueError>(py));
 #     })
 # }
 ```
@@ -257,5 +255,5 @@ defines exceptions for several standard library modules.
 [`PyErr`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyErr.html
 [`PyResult`]: {{#PYO3_DOCS_URL}}/pyo3/type.PyResult.html
 [`PyErr::from_instance`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyErr.html#method.from_instance
-[`Python::is_instance`]: {{#PYO3_DOCS_URL}}/pyo3/struct.Python.html#method.is_instance
 [`PyAny::is_instance`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyAny.html#method.is_instance
+[`PyAny::is_instance_of`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyAny.html#method.is_instance_of

src/conversions/path.rs

@@ -20,7 +20,7 @@ impl FromPyObject<'_> for PathBuf {
                 let py = ob.py();
                 let pathlib = py.import("pathlib")?;
                 let pathlib_path: &PyType = pathlib.getattr("Path")?.downcast()?;
-                if pathlib_path.is_instance(ob)? {
+                if ob.is_instance(pathlib_path)? {
                     let path_str = ob.call_method0("__str__")?;
                     OsString::extract(path_str)?
                 } else {

src/err/mod.rs

@@ -180,7 +180,7 @@ impl PyErr {
     ///
     /// Python::with_gil(|py| {
     ///     let err: PyErr = PyTypeError::new_err(("some type error",));
-    ///     assert!(err.is_instance::<PyTypeError>(py));
+    ///     assert!(err.is_instance_of::<PyTypeError>(py));
     ///     assert_eq!(err.pvalue(py).to_string(), "some type error");
     /// });
     /// ```
@@ -366,13 +366,16 @@ impl PyErr {
     }
 
     /// Returns true if the current exception is instance of `T`.
-    pub fn is_instance<T>(&self, py: Python) -> bool
+    pub fn is_instance(&self, py: Python, typ: &PyType) -> bool {
+        unsafe { ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(py), typ.as_ptr()) != 0 }
+    }
+
+    /// Returns true if the current exception is instance of `T`.
+    pub fn is_instance_of<T>(&self, py: Python) -> bool
     where
         T: PyTypeObject,
     {
-        unsafe {
+        self.is_instance(py, T::type_object(py))
-            ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(py), T::type_object(py).as_ptr()) != 0
-        }
     }
 
     /// Retrieves the exception instance for this error.
@@ -612,11 +615,11 @@ mod tests {
     fn set_valueerror() {
         Python::with_gil(|py| {
             let err: PyErr = exceptions::PyValueError::new_err("some exception message");
-            assert!(err.is_instance::<exceptions::PyValueError>(py));
+            assert!(err.is_instance_of::<exceptions::PyValueError>(py));
             err.restore(py);
             assert!(PyErr::occurred(py));
             let err = PyErr::fetch(py);
-            assert!(err.is_instance::<exceptions::PyValueError>(py));
+            assert!(err.is_instance_of::<exceptions::PyValueError>(py));
             assert_eq!(err.to_string(), "ValueError: some exception message");
         })
     }
@@ -625,10 +628,10 @@ mod tests {
     fn invalid_error_type() {
         Python::with_gil(|py| {
             let err: PyErr = PyErr::new::<crate::types::PyString, _>(());
-            assert!(err.is_instance::<exceptions::PyTypeError>(py));
+            assert!(err.is_instance_of::<exceptions::PyTypeError>(py));
             err.restore(py);
             let err = PyErr::fetch(py);
-            assert!(err.is_instance::<exceptions::PyTypeError>(py));
+            assert!(err.is_instance_of::<exceptions::PyTypeError>(py));
             assert_eq!(
                 err.to_string(),
                 "TypeError: exceptions must derive from BaseException"

src/exceptions.rs

@@ -267,7 +267,7 @@ fn always_throws() -> PyResult<()> {
 # Python::with_gil(|py| {
 #     let fun = pyo3::wrap_pyfunction!(always_throws, py).unwrap();
 #     let err = fun.call0().expect_err(\"called a function that should always return an error but the return value was Ok\");
-#     assert!(err.is_instance::<Py", $name, ">(py))
+#     assert!(err.is_instance_of::<Py", $name, ">(py))
 # });
 ```
 
@@ -292,7 +292,7 @@ Python::with_gil(|py| {
 
     let error_type = match result {
         Ok(_) => \"Not an error\",
-        Err(error) if error.is_instance::<Py", $name, ">(py) => \"" , $name, "\",
+        Err(error) if error.is_instance_of::<Py", $name, ">(py) => \"" , $name, "\",
         Err(_) => \"Some other error\",
     };
 
@@ -611,7 +611,7 @@ macro_rules! test_exception {
                         .unwrap_or($exc_ty::new_err("a test exception"))
                 };
 
-                assert!(err.is_instance::<$exc_ty>(py));
+                assert!(err.is_instance_of::<$exc_ty>(py));
 
                 let value: &$exc_ty = err.instance(py).downcast().unwrap();
                 assert!(value.source().is_none());
@@ -619,7 +619,7 @@ macro_rules! test_exception {
                 err.set_cause(py, Some($crate::exceptions::PyValueError::new_err("a cause")));
                 assert!(value.source().is_some());
 
-                assert!($crate::PyErr::from(value).is_instance::<$exc_ty>(py));
+                assert!($crate::PyErr::from(value).is_instance_of::<$exc_ty>(py));
             })
         }
     };

src/types/any.rs

@@ -74,7 +74,7 @@ impl PyAny {
     ///
     /// Python::with_gil(|py| {
     ///     let dict = PyDict::new(py);
-    ///     assert!(dict.is_instance::<PyAny>().unwrap());
+    ///     assert!(dict.is_instance_of::<PyAny>().unwrap());
     ///     let any: &PyAny = dict.as_ref();
     ///     assert!(any.downcast::<PyDict>().is_ok());
     ///     assert!(any.downcast::<PyList>().is_err());
@@ -665,11 +665,21 @@ impl PyAny {
         unsafe { self.py().from_owned_ptr(ffi::PyObject_Dir(self.as_ptr())) }
     }
 
+    /// Checks whether this object is an instance of type `typ`.
+    ///
+    /// This is equivalent to the Python expression `isinstance(self, typ)`.
+    pub fn is_instance(&self, typ: &PyType) -> PyResult<bool> {
+        let result = unsafe { ffi::PyObject_IsInstance(self.as_ptr(), typ.as_ptr()) };
+        err::error_on_minusone(self.py(), result)?;
+        Ok(result == 1)
+    }
+
     /// Checks whether this object is an instance of type `T`.
     ///
-    /// This is equivalent to the Python expression `isinstance(self, T)`.
+    /// This is equivalent to the Python expression `isinstance(self, T)`,
-    pub fn is_instance<T: PyTypeObject>(&self) -> PyResult<bool> {
+    /// if the type `T` is known at compile time.
-        T::type_object(self.py()).is_instance(self)
+    pub fn is_instance_of<T: PyTypeObject>(&self) -> PyResult<bool> {
+        self.is_instance(T::type_object(self.py()))
     }
 
     /// Returns a GIL marker constrained to the lifetime of this type.
@@ -682,6 +692,7 @@ impl PyAny {
 #[cfg(test)]
 mod tests {
     use crate::{
+        type_object::PyTypeObject,
         types::{IntoPyDict, PyList, PyLong, PyModule},
         Python, ToPyObject,
     };
@@ -776,10 +787,18 @@ mod tests {
     fn test_any_isinstance() {
         Python::with_gil(|py| {
             let x = 5.to_object(py).into_ref(py);
-            assert!(x.is_instance::<PyLong>().unwrap());
+            assert!(x.is_instance_of::<PyLong>().unwrap());
 
             let l = vec![x, x].to_object(py).into_ref(py);
-            assert!(l.is_instance::<PyList>().unwrap());
+            assert!(l.is_instance_of::<PyList>().unwrap());
+        });
+    }
+
+    #[test]
+    fn test_any_isinstance_of() {
+        Python::with_gil(|py| {
+            let l = vec![1u8, 2].to_object(py).into_ref(py);
+            assert!(l.is_instance(PyList::type_object(py)).unwrap());
         });
     }
 }

src/types/bytearray.rs

@@ -241,7 +241,7 @@ mod tests {
     fn test_from_err() {
         Python::with_gil(|py| {
             if let Err(err) = PyByteArray::from(py, &py.None()) {
-                assert!(err.is_instance::<exceptions::PyTypeError>(py));
+                assert!(err.is_instance_of::<exceptions::PyTypeError>(py));
             } else {
                 panic!("error");
             }
@@ -293,7 +293,7 @@ mod tests {
             assert!(py_bytearray_result
                 .err()
                 .unwrap()
-                .is_instance::<PyValueError>(py));
+                .is_instance_of::<PyValueError>(py));
         })
     }
 }

src/types/bytes.rs

@@ -174,7 +174,7 @@ mod tests {
             assert!(py_bytes_result
                 .err()
                 .unwrap()
-                .is_instance::<PyValueError>(py));
+                .is_instance_of::<PyValueError>(py));
         });
     }
 }

src/types/iterator.rs

@@ -206,7 +206,7 @@ mod tests {
             let x = 5.to_object(py);
             let err = PyIterator::from_object(py, &x).unwrap_err();
 
-            assert!(err.is_instance::<PyTypeError>(py));
+            assert!(err.is_instance_of::<PyTypeError>(py));
         });
     }
 

src/types/mapping.rs

@@ -180,7 +180,7 @@ mod tests {
             assert!(mapping
                 .get_item(8i32)
                 .unwrap_err()
-                .is_instance::<PyKeyError>(py));
+                .is_instance_of::<PyKeyError>(py));
         });
     }
 
@@ -216,7 +216,7 @@ mod tests {
             assert!(mapping
                 .get_item(7i32)
                 .unwrap_err()
-                .is_instance::<PyKeyError>(py));
+                .is_instance_of::<PyKeyError>(py));
         });
     }
 

src/types/module.rs

@@ -146,7 +146,7 @@ impl PyModule {
         match self.getattr("__all__") {
             Ok(idx) => idx.downcast().map_err(PyErr::from),
             Err(err) => {
-                if err.is_instance::<exceptions::PyAttributeError>(self.py()) {
+                if err.is_instance_of::<exceptions::PyAttributeError>(self.py()) {
                     let l = PyList::empty(self.py());
                     self.setattr("__all__", l).map_err(PyErr::from)?;
                     Ok(l)

src/types/num.rs

@@ -345,7 +345,7 @@ mod test_128bit_intergers {
         Python::with_gil(|py| {
             let obj = py.eval("(1 << 130) * -1", None, None).unwrap();
             let err = obj.extract::<i128>().unwrap_err();
-            assert!(err.is_instance::<crate::exceptions::PyOverflowError>(py));
+            assert!(err.is_instance_of::<crate::exceptions::PyOverflowError>(py));
         })
     }
 
@@ -354,7 +354,7 @@ mod test_128bit_intergers {
         Python::with_gil(|py| {
             let obj = py.eval("1 << 130", None, None).unwrap();
             let err = obj.extract::<u128>().unwrap_err();
-            assert!(err.is_instance::<crate::exceptions::PyOverflowError>(py));
+            assert!(err.is_instance_of::<crate::exceptions::PyOverflowError>(py));
         })
     }
 }
@@ -421,7 +421,7 @@ mod tests {
 
                     let obj = ("123").to_object(py);
                     let err = obj.extract::<$t>(py).unwrap_err();
-                    assert!(err.is_instance::<exceptions::PyTypeError>(py));
+                    assert!(err.is_instance_of::<exceptions::PyTypeError>(py));
                     });
                 }
 
@@ -431,7 +431,7 @@ mod tests {
 
                     let obj = (12.3).to_object(py);
                     let err = obj.extract::<$t>(py).unwrap_err();
-                    assert!(err.is_instance::<exceptions::PyTypeError>(py));});
+                    assert!(err.is_instance_of::<exceptions::PyTypeError>(py));});
                 }
 
                 #[test]

src/types/typeobject.rs

@@ -40,25 +40,48 @@ impl PyType {
         self.getattr("__qualname__")?.extract()
     }
 
-    /// Checks whether `self` is subclass of type `T`.
+    /// Checks whether `self` is a subclass of `other`.
     ///
-    /// Equivalent to Python's `issubclass` function.
+    /// Equivalent to the Python expression `issubclass(self, other)`.
-    pub fn is_subclass<T>(&self) -> PyResult<bool>
+    pub fn is_subclass(&self, other: &PyType) -> PyResult<bool> {
-    where
+        let result = unsafe { ffi::PyObject_IsSubclass(self.as_ptr(), other.as_ptr()) };
-        T: PyTypeObject,
-    {
-        let result =
-            unsafe { ffi::PyObject_IsSubclass(self.as_ptr(), T::type_object(self.py()).as_ptr()) };
         err::error_on_minusone(self.py(), result)?;
         Ok(result == 1)
     }
 
-    /// Check whether `obj` is an instance of `self`.
+    /// Checks whether `self` is a subclass of type `T`.
     ///
-    /// Equivalent to Python's `isinstance` function.
+    /// Equivalent to the Python expression `issubclass(self, T)`, if the type
-    pub fn is_instance<T: AsPyPointer>(&self, obj: &T) -> PyResult<bool> {
+    /// `T` is known at compile time.
-        let result = unsafe { ffi::PyObject_IsInstance(obj.as_ptr(), self.as_ptr()) };
+    pub fn is_subclass_of<T>(&self) -> PyResult<bool>
-        err::error_on_minusone(self.py(), result)?;
+    where
-        Ok(result == 1)
+        T: PyTypeObject,
+    {
+        self.is_subclass(T::type_object(self.py()))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::{
+        type_object::PyTypeObject,
+        types::{PyBool, PyLong},
+        Python,
+    };
+
+    #[test]
+    fn test_type_is_subclass() {
+        Python::with_gil(|py| {
+            let bool_type = PyBool::type_object(py);
+            let long_type = PyLong::type_object(py);
+            assert!(bool_type.is_subclass(long_type).unwrap());
+        });
+    }
+
+    #[test]
+    fn test_type_is_subclass_of() {
+        Python::with_gil(|py| {
+            assert!(PyBool::type_object(py).is_subclass_of::<PyLong>().unwrap());
+        });
     }
 }

tests/test_inheritance.rs

@@ -1,5 +1,6 @@
 use pyo3::prelude::*;
 use pyo3::py_run;
+use pyo3::type_object::PyTypeObject;
 
 use pyo3::types::IntoPyDict;
 
@@ -102,6 +103,23 @@ fn mutation_fails() {
     assert_eq!(&e.to_string(), "RuntimeError: Already borrowed")
 }
 
+#[test]
+fn is_subclass_and_is_instance() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let sub_ty = SubClass::type_object(py);
+    let base_ty = BaseClass::type_object(py);
+    assert!(sub_ty.is_subclass_of::<BaseClass>().unwrap());
+    assert!(sub_ty.is_subclass(base_ty).unwrap());
+
+    let obj = PyCell::new(py, SubClass::new()).unwrap();
+    assert!(obj.is_instance_of::<SubClass>().unwrap());
+    assert!(obj.is_instance_of::<BaseClass>().unwrap());
+    assert!(obj.is_instance(sub_ty).unwrap());
+    assert!(obj.is_instance(base_ty).unwrap());
+}
+
 #[pyclass(subclass)]
 struct BaseClassWithResult {
     _val: usize,

tests/test_proto_methods.rs

@@ -97,7 +97,7 @@ fn test_getattr() {
         assert!(example_py
             .getattr("other_attr")
             .unwrap_err()
-            .is_instance::<PyAttributeError>(py));
+            .is_instance_of::<PyAttributeError>(py));
     })
 }