Add FromPyObject impl for Cow<Path> & Cow<OsStr> #5497
Conversation
CodSpeed Performance ReportMerging #5497 will not alter performanceComparing Summary
|
bschoenmaeckers force-pushed the extract_cow_path_osstr branch from 178bd02 to b438e54 Compare 
davidhewitt left a comment
davidhewitt left a comment There was a problem hiding this comment.
Good idea, with possibly more to go 🤔
| if let Ok(s) = obj.extract::<&str>() { | ||
| return Ok(Cow::Borrowed(s.as_ref())); | ||
| } | ||
| | ||
| obj.extract::<OsString>().map(Cow::Owned) |
There was a problem hiding this comment.
Based on the above comment about &OsStr, I think we might be able to make this always not copy on unix platforms?
I'm unsure if this comment is correct. |
| Ah right, makes sense. Willing to delete that comment in this PR perhaps? Otherwise LGTM! |
bschoenmaeckers force-pushed the extract_cow_path_osstr branch from b438e54 to 44b3311 Compare davidhewitt left a comment
davidhewitt left a comment There was a problem hiding this comment.
Actually sorry quick suggestion for the tests.
src/conversions/std/osstr.rs Outdated
| } | ||
| | ||
| // Test extracting both valid UTF-8 and non-UTF-8 strings | ||
| test_extract(py, "Hello\0\n🐍"); |
There was a problem hiding this comment.
Actually shall we assert here that this is the borrowed form as expected, and similar for the path tests?
Will need a cfg but not too bad.
There was a problem hiding this comment.
No problem! Will add this to the test!
bschoenmaeckers force-pushed the extract_cow_path_osstr branch 3 times, most recently from f9d1676 to 3f5e731 Compare bschoenmaeckers force-pushed the extract_cow_path_osstr branch from 3f5e731 to cf72c95 Compare 2 participants
Allow extracting a
PathandOsStrwithout copying when it is a valid utf8 string.