Skip to content
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

Python bindings #28

Closed
wants to merge 7 commits into from
Closed

Python bindings #28

wants to merge 7 commits into from

Conversation

elshize
Copy link
Member

@elshize elshize commented May 20, 2021

Still work in progress.

Things left to do:

  • Investigate how to make the functions to accept string-like or path-like types instead of a string.
  • Set up CI
  • Figure out how to easily distribute the python package

@elshize elshize self-assigned this May 20, 2021
@JMMackenzie
Copy link
Member

Awesome work so far!

Regarding (1), perhaps this can help? PyO3/pyo3#1377

@elshize
Copy link
Member Author

elshize commented May 23, 2021

Awesome work so far!

Regarding (1), perhaps this can help? PyO3/pyo3#1377

This doesn't really help us. First, it's not released yet (not a big issue though). But more importantly, it allows us to pass str from python and automatically convert to say PathBuf so we don't need that explicit conversion. But it doesn't help with someone passing non-string variable to us.

I solved it by having a wrapper function in the python code that casts all arguments to strings. So whatever the user passes, it will work as long as it converts to string. Additionally, any conversion problems will be handled in python, so the user will get some familiar message without Py* types printed out. So I think that's the way to go.

@JMMackenzie
Copy link
Member

@elshize I think this is dead now right?

@elshize elshize closed this Mar 8, 2022
@elshize elshize deleted the python branch March 8, 2022 23:51
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 this pull request may close these issues.

2 participants