Convert PathBuf & Path into python pathlib.Path instead of `PyS…

…tring` (#4925) * Convert `PathBuf` into python `pathlib.Path` * Make sure `ToPyObject` & `IntoPy` convert to PyString * Add from pystring test
PyO3 · Feb 26, 2025 · ffd7276 · ffd7276
1 parent b07871d
commit ffd7276
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 36 deletions.
diff --git a/newsfragments/4925.changed.md b/newsfragments/4925.changed.md
@@ -0,0 +1 @@
+Convert `PathBuf` & `Path` into python `pathlib.Path` instead of `PyString`
diff --git a/pytests/tests/test_path.py b/pytests/tests/test_path.py
@@ -7,21 +7,21 @@
 
 def test_make_path():
     p = rpath.make_path()
-    assert p == "/root"
+    assert p == pathlib.Path("/root")
 
 
 def test_take_pathbuf():
     p = "/root"
-    assert rpath.take_pathbuf(p) == p
+    assert rpath.take_pathbuf(p) == pathlib.Path(p)
 
 
 def test_take_pathlib():
     p = pathlib.Path("/root")
-    assert rpath.take_pathbuf(p) == str(p)
+    assert rpath.take_pathbuf(p) == p
 
 
 def test_take_pathlike():
-    assert rpath.take_pathbuf(PathLike("/root")) == "/root"
+    assert rpath.take_pathbuf(PathLike("/root")) == pathlib.Path("/root")
 
 
 def test_take_invalid_pathlike():

diff --git a/src/conversions/std/path.rs b/src/conversions/std/path.rs
@@ -1,21 +1,20 @@
 use crate::conversion::IntoPyObject;
 use crate::ffi_ptr_ext::FfiPtrExt;
 use crate::instance::Bound;
+use crate::sync::GILOnceCell;
 use crate::types::any::PyAnyMethods;
-use crate::types::PyString;
-use crate::{ffi, FromPyObject, PyAny, PyObject, PyResult, Python};
+use crate::{ffi, FromPyObject, IntoPyObjectExt, PyAny, PyErr, PyObject, PyResult, Python};
 #[allow(deprecated)]
 use crate::{IntoPy, ToPyObject};
 use std::borrow::Cow;
-use std::convert::Infallible;
 use std::ffi::OsString;
 use std::path::{Path, PathBuf};
 
 #[allow(deprecated)]
 impl ToPyObject for Path {
     #[inline]
     fn to_object(&self, py: Python<'_>) -> PyObject {
-        self.into_pyobject(py).unwrap().into_any().unbind()
+        self.as_os_str().into_py_any(py).unwrap()
     }
 }
 
@@ -33,25 +32,28 @@ impl FromPyObject<'_> for PathBuf {
 impl IntoPy<PyObject> for &Path {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
-        self.into_pyobject(py).unwrap().into_any().unbind()
+        self.to_object(py)
     }
 }
 
 impl<'py> IntoPyObject<'py> for &Path {
-    type Target = PyString;
+    type Target = PyAny;
     type Output = Bound<'py, Self::Target>;
-    type Error = Infallible;
+    type Error = PyErr;
 
     #[inline]
     fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
-        self.as_os_str().into_pyobject(py)
+        static PY_PATH: GILOnceCell<PyObject> = GILOnceCell::new();
+        PY_PATH
+            .import(py, "pathlib", "Path")?
+            .call((self.as_os_str(),), None)
     }
 }
 
 impl<'py> IntoPyObject<'py> for &&Path {
-    type Target = PyString;
+    type Target = PyAny;
     type Output = Bound<'py, Self::Target>;
-    type Error = Infallible;
+    type Error = PyErr;
 
     #[inline]
     fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
@@ -63,90 +65,90 @@ impl<'py> IntoPyObject<'py> for &&Path {
 impl ToPyObject for Cow<'_, Path> {
     #[inline]
     fn to_object(&self, py: Python<'_>) -> PyObject {
-        self.into_pyobject(py).unwrap().into_any().unbind()
+        (**self).to_object(py)
     }
 }
 
 #[allow(deprecated)]
 impl IntoPy<PyObject> for Cow<'_, Path> {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
-        self.into_pyobject(py).unwrap().into_any().unbind()
+        (*self).to_object(py)
     }
 }
 
 impl<'py> IntoPyObject<'py> for Cow<'_, Path> {
-    type Target = PyString;
+    type Target = PyAny;
     type Output = Bound<'py, Self::Target>;
-    type Error = Infallible;
+    type Error = PyErr;
 
     #[inline]
     fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
-        self.as_os_str().into_pyobject(py)
+        (*self).into_pyobject(py)
     }
 }
 
 impl<'py> IntoPyObject<'py> for &Cow<'_, Path> {
-    type Target = PyString;
+    type Target = PyAny;
     type Output = Bound<'py, Self::Target>;
-    type Error = Infallible;
+    type Error = PyErr;
 
     #[inline]
     fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
-        self.as_os_str().into_pyobject(py)
+        (&**self).into_pyobject(py)
     }
 }
 
 #[allow(deprecated)]
 impl ToPyObject for PathBuf {
     #[inline]
     fn to_object(&self, py: Python<'_>) -> PyObject {
-        self.into_pyobject(py).unwrap().into_any().unbind()
+        (**self).to_object(py)
     }
 }
 
 #[allow(deprecated)]
 impl IntoPy<PyObject> for PathBuf {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
-        self.into_pyobject(py).unwrap().into_any().unbind()
+        (*self).to_object(py)
     }
 }
 
 impl<'py> IntoPyObject<'py> for PathBuf {
-    type Target = PyString;
+    type Target = PyAny;
     type Output = Bound<'py, Self::Target>;
-    type Error = Infallible;
+    type Error = PyErr;
 
     #[inline]
     fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
-        self.as_os_str().into_pyobject(py)
+        (&self).into_pyobject(py)
     }
 }
 
 #[allow(deprecated)]
 impl IntoPy<PyObject> for &PathBuf {
     #[inline]
     fn into_py(self, py: Python<'_>) -> PyObject {
-        self.into_pyobject(py).unwrap().into_any().unbind()
+        (**self).to_object(py)
     }
 }
 
 impl<'py> IntoPyObject<'py> for &PathBuf {
-    type Target = PyString;
+    type Target = PyAny;
     type Output = Bound<'py, Self::Target>;
-    type Error = Infallible;
+    type Error = PyErr;
 
     #[inline]
     fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
-        self.as_os_str().into_pyobject(py)
+        (&**self).into_pyobject(py)
     }
 }
 
 #[cfg(test)]
 mod tests {
     use crate::types::{PyAnyMethods, PyString, PyStringMethods};
-    use crate::{BoundObject, IntoPyObject, Python};
+    use crate::{IntoPyObject, IntoPyObjectExt, PyObject, Python};
     use std::borrow::Cow;
     use std::fmt::Debug;
     use std::path::{Path, PathBuf};
@@ -180,10 +182,42 @@ mod tests {
                 T: IntoPyObject<'py> + AsRef<Path> + Debug + Clone,
                 T::Error: Debug,
             {
-                let pyobject = obj.clone().into_pyobject(py).unwrap().into_any();
-                let pystring = pyobject.as_borrowed().downcast::<PyString>().unwrap();
+                let pyobject = obj.clone().into_bound_py_any(py).unwrap();
+                let roundtripped_obj: PathBuf = pyobject.extract().unwrap();
+                assert_eq!(obj.as_ref(), roundtripped_obj.as_path());
+            }
+            let path = Path::new("Hello\0\n🐍");
+            test_roundtrip::<&Path>(py, path);
+            test_roundtrip::<Cow<'_, Path>>(py, Cow::Borrowed(path));
+            test_roundtrip::<Cow<'_, Path>>(py, Cow::Owned(path.to_path_buf()));
+            test_roundtrip::<PathBuf>(py, path.to_path_buf());
+        });
+    }
+
+    #[test]
+    fn test_from_pystring() {
+        Python::with_gil(|py| {
+            let path = "Hello\0\n🐍";
+            let pystring = PyString::new(py, path);
+            let roundtrip: PathBuf = pystring.extract().unwrap();
+            assert_eq!(roundtrip, Path::new(path));
+        });
+    }
+
+    #[test]
+    #[allow(deprecated)]
+    fn test_intopy_string() {
+        use crate::IntoPy;
+
+        Python::with_gil(|py| {
+            fn test_roundtrip<T>(py: Python<'_>, obj: T)
+            where
+                T: IntoPy<PyObject> + AsRef<Path> + Debug + Clone,
+            {
+                let pyobject = obj.clone().into_py(py).into_bound(py);
+                let pystring = pyobject.downcast_exact::<PyString>().unwrap();
                 assert_eq!(pystring.to_string_lossy(), obj.as_ref().to_string_lossy());
-                let roundtripped_obj: PathBuf = pystring.extract().unwrap();
+                let roundtripped_obj: PathBuf = pyobject.extract().unwrap();
                 assert_eq!(obj.as_ref(), roundtripped_obj.as_path());
             }
             let path = Path::new("Hello\0\n🐍");

diff --git a/tests/test_datetime_import.rs b/tests/test_datetime_import.rs
@@ -18,7 +18,7 @@ fn test_bad_datetime_module_panic() {
         let sys = py.import("sys").unwrap();
         sys.getattr("path")
             .unwrap()
-            .call_method1("insert", (0, tmpdir.path()))
+            .call_method1("insert", (0, tmpdir.path().as_os_str()))
             .unwrap();
 
         // This should panic because the "datetime" module is empty
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Convert `PathBuf` & `Path` into python `pathlib.Path` instead of `PyString`