Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implement IntoPy<PyObject> for &PathBuf and &OsString #1712

Merged
merged 1 commit into from
Jul 4, 2021

Conversation

messense
Copy link
Member

@messense messense commented Jul 4, 2021

Without this impl users need to call as_path() to convert a &PathBuf to a &Path, for example messense/rjsonnet-py@b29e161

Otherwise the error message is not easy to understand:

error[E0277]: the trait bound `PathBuf: AsPyPointer` is not satisfied
  --> src/lib.rs:34:55
   |
34 |             Python::with_gil(|py| match self.callback.call(py, (from, path), None) {
   |                                                       ^^^^ the trait `AsPyPointer` is not implemented for `PathBuf`
   |
   = note: required because of the requirements on the impl of `pyo3::IntoPy<Py<PyAny>>` for `&PathBuf`
   = note: 1 redundant requirements hidden
   = note: required because of the requirements on the impl of `pyo3::IntoPy<Py<PyTuple>>` for `(&PathBuf, &PathBuf)`

@messense messense force-pushed the pathbuf-into-py branch 2 times, most recently from 11d7d66 to 3df773d Compare July 4, 2021 06:19
@davidhewitt
Copy link
Member

davidhewitt commented Jul 4, 2021

👍 Can you add it for &OsString too please? That would then be consistent with &String, which already exists. Otherwise LGTM!

Btw I am preparing a 0.14.1 release today to fix a PyPy crash found in pyca/cryptography#6154, happy to see this PR merged and included in the release too.

@messense
Copy link
Member Author

messense commented Jul 4, 2021

👍 Can you add it for &OsString too please? That would then be consistent with &String, which already exists.

Added.

@messense messense changed the title Implement IntoPy<PyObject> for &PathBuf Implement IntoPy<PyObject> for &PathBuf and &OsString Jul 4, 2021
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great, thanks!

@messense messense merged commit 9cdb6a0 into PyO3:main Jul 4, 2021
@messense messense deleted the pathbuf-into-py branch July 4, 2021 08:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants